fix(proxy): improve Headroom /v1/compress HTTP 404 diagnostics - #35952
Conversation
Greptile SummaryThis PR centralizes Headroom
Confidence Score: 4/5The PR appears safe to merge after addressing the non-blocking mutable dictionary construction The changed failure paths preserve existing status and body diagnostics, conditionally add the intended 404 hint, and have focused regression coverage; the only accepted concern is a repository coding-convention violation Files Needing Attention: litellm/proxy/guardrails/guardrail_hooks/headroom/headroom.py
|
| Filename | Overview |
|---|---|
| litellm/proxy/guardrails/guardrail_hooks/headroom/headroom.py | Adds shared 404 diagnostic construction; behavior is consistent across both failure branches, but the helper violates the repository's no-mutation convention |
| tests/test_litellm/proxy/guardrails/guardrail_hooks/test_headroom.py | Adds focused mocked regression tests confirming that 404 responses include the hint and non-404 responses preserve the prior detail shape |
Reviews (1): Last reviewed commit: "fix(proxy): improve Headroom 404 compres..." | Re-trigger Greptile
| detail: dict[str, object] = {"status_code": status_code, "body": body} | ||
| if status_code == 404: | ||
| detail["hint"] = ( | ||
| "The Headroom compression endpoint returned HTTP 404. " | ||
| "Verify that the configured Headroom endpoint is correct and that " | ||
| "the compression endpoint is available. If you are using a " | ||
| "self-hosted deployment, some deployments require enabling remote " | ||
| "compression (for example, HEADROOM_COMPRESS_ALLOW_REMOTE=1)." | ||
| ) | ||
| return detail |
There was a problem hiding this comment.
Avoid mutable detail construction
The helper initializes detail and then mutates it to add the hint, contrary to the repository's functional coding convention. Constructing and returning the appropriate dictionary in each branch avoids unnecessary mutable state.
| detail: dict[str, object] = {"status_code": status_code, "body": body} | |
| if status_code == 404: | |
| detail["hint"] = ( | |
| "The Headroom compression endpoint returned HTTP 404. " | |
| "Verify that the configured Headroom endpoint is correct and that " | |
| "the compression endpoint is available. If you are using a " | |
| "self-hosted deployment, some deployments require enabling remote " | |
| "compression (for example, HEADROOM_COMPRESS_ALLOW_REMOTE=1)." | |
| ) | |
| return detail | |
| if status_code == 404: | |
| return { | |
| "status_code": status_code, | |
| "body": body, | |
| "hint": ( | |
| "The Headroom compression endpoint returned HTTP 404. " | |
| "Verify that the configured Headroom endpoint is correct and that " | |
| "the compression endpoint is available. If you are using a " | |
| "self-hosted deployment, some deployments require enabling remote " | |
| "compression (for example, HEADROOM_COMPRESS_ALLOW_REMOTE=1)." | |
| ), | |
| } | |
| return {"status_code": status_code, "body": body} |
Context Used: CLAUDE.md (source)
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!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@aayush598 to fix the lint fail, pull latest staging. I've since updated the error message to be more helpful (but, it's kind of a chicken-and-egg problem since you need to pull staging to see the improved error message :P) |
932c4c4 to
29d52cd
Compare
|
Hi @mateo-berri — rebased onto latest litellm_internal_staging (4b7adab) and applied the Greptile suggestion. All 79 CI checks pass (lint, unit tests, CodSpeed, Veria AI) and Greptile confidence is 4/5. Could you take a look and approve when you get a chance? Thanks! |
d332acc
into
BerriAI:litellm_internal_staging
|
thanks! |
TLDR
Problem this solves:
/v1/compressHTTP 404 responses provide limited troubleshooting context.How it solves it:
/v1/compressHTTP 404 responses.Relevant issues
Related to #35933
Linear ticket
N/A
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Regression tests added:
test_apply_guardrail_404_error_includes_troubleshooting_hinttest_apply_guardrail_non_404_error_omits_troubleshooting_hintThe tests verify:
The behavior change is limited to the error payload returned for HTTP 404 responses.
Type
🐛 Bug Fix
✅ Test
Changes
_build_compress_failure_detail()to centralize/v1/compresserror detail construction.status_codeandbodyfor all responses./v1/compresserror paths to use the shared helper.Final Attestation