Fix problems with callers confused by gzip encoding#2
Fix problems with callers confused by gzip encoding#2ropoctl wants to merge 2 commits intoQuixiAI:mainfrom
Conversation
When caller allows gzip encoding, and the backend sends content-encoding gzip, we either need to recompress, or pop the header for the caller.
📝 WalkthroughWalkthroughResponse header handling in the proxy's non-streaming path was changed to use a mutable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
proxy.py (1)
290-293: Consider aligning with the streaming path's approach.The streaming path (lines 232-233) removes these headers using a dict comprehension. For consistency, you could use the same pattern here, though both approaches work correctly.
🔎 Alternative approach matching streaming pattern
- mutable_headers = CIMultiDict(upstream_response.headers) - mutable_headers.pop('Transfer-Encoding', None) - mutable_headers.pop('Content-Encoding', None) - + response_headers = { + key: value + for key, value in upstream_response.headers.items() + if key.lower() not in ['content-length', 'transfer-encoding', 'content-encoding'] + } + return web.Response( status=upstream_response.status, body=response_body, - headers=mutable_headers, + headers=response_headers, )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
proxy.py
🧰 Additional context used
🪛 Ruff (0.14.10)
proxy.py
290-290: Undefined name CIMultiDict
(F821)
🔇 Additional comments (1)
proxy.py (1)
27-27: LGTM!The yaml import is correctly placed and used for configuration loading.
| mutable_headers = CIMultiDict(upstream_response.headers) | ||
| mutable_headers.pop('Transfer-Encoding', None) | ||
| mutable_headers.pop('Content-Encoding', None) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, get an overview of the proxy.py file structure and handler locations
head -300 proxy.py | tail -50Repository: QuixiAI/VibeLogger
Length of output: 2675
🏁 Script executed:
# Look at the specific section around lines 290-293
sed -n '285,305p' proxy.pyRepository: QuixiAI/VibeLogger
Length of output: 968
🏁 Script executed:
# Check the structure of other handlers mentioned
sed -n '325,345p' proxy.py
sed -n '455,475p' proxy.py
sed -n '620,640p' proxy.pyRepository: QuixiAI/VibeLogger
Length of output: 2644
Apply the same header cleanup to other proxy handlers for consistency.
The fix at lines 290-293 correctly removes Transfer-Encoding and Content-Encoding headers from the Gemini response. However, the same cleanup should be applied to handle_list_models, handle_anthropic_request, and handle_openai_request, which currently pass upstream headers directly without removing these encoding headers. If the gzip encoding issue affects proxied responses, it affects all handlers and should be handled consistently.
🧰 Tools
🪛 Ruff (0.14.10)
290-290: Undefined name CIMultiDict
(F821)
🤖 Prompt for AI Agents
In proxy.py around lines 290-293 and in the handlers handle_list_models,
handle_anthropic_request, and handle_openai_request, the upstream response
headers are currently forwarded as-is; replicate the same header cleanup used at
lines 290-293 by creating a mutable copy of upstream_response.headers, removing
'Transfer-Encoding' and 'Content-Encoding' with pop(..., None), and use that
cleaned headers mapping when constructing the proxied response so all proxy
handlers consistently strip those encoding headers; also handle cases where
upstream_response or headers may be missing before copying.
1 participant
When caller allows gzip encoding, and the backend sends content-encoding gzip, we either need to recompress, or pop the header for the caller.
nb: multidict is not a new dependency, its already the type aiohttp uses; including that in the uv import shebang for hygiene.
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.