Pool tokens for embed image#7
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDocumentation and code updates add explicit embed_image protocol details (image.tobytes(), LZ4 frame, x-output-dtype, x-output-shape) and extend Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request introduces support for custom headers in the embed_image method for both synchronous and asynchronous clients. It updates the documentation to explain the serialization and compression process, and modifies the implementation to convert keyword arguments into x- prefixed headers. The review feedback correctly identifies that the docstring examples in models.py are inconsistent with the code logic, as they omit the x- prefix in the illustrated output.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds support for passing additional x-* request headers to the embed_image model endpoint, enabling server-side options (e.g., token pooling) to be controlled from the SDK.
Changes:
- Extend
Models.embed_image/AsyncModels.embed_imageto accept**headersand send them asx-*request headers (underscores → hyphens). - Update the “How it works” documentation to describe embed-image request/response behavior, including the new header mechanism and
x-output-shape.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
rationai/resources/models.py |
Adds **headers support to embed-image requests and constructs a merged request headers dict. |
docs/learn/how-it-works.md |
Documents embed-image wire format, including x-output-dtype, optional x-* keyword headers, and x-output-shape. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
rationai/resources/models.py (1)
91-94: Consider extracting header-building into a shared helper.The sync/async methods duplicate the same header normalization block; centralizing it reduces drift risk.
Also applies to: 190-193
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rationai/resources/models.py` around lines 91 - 94, The header normalization logic that builds request_headers (using np.dtype(output_dtype).name and mapping headers via f"x-{k.replace('_', '-')"} is duplicated between the sync and async request methods; extract this into a single helper function (e.g., build_request_headers or normalize_request_headers) that takes output_dtype and headers and returns the normalized dict, then replace the inline blocks in the methods that currently assign request_headers with calls to that helper (ensure the helper imports numpy as np or accepts already-normalized dtype string). This centralizes the logic and removes duplication referenced by the request_headers usage in the existing methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rationai/resources/models.py`:
- Around line 82-84: Update the docstring examples that describe header
handling: change the example showing "pool-tokens: false" to include the x-
prefix (e.g., "x-pool-tokens: false") so it matches the implementation which
emits "x-" + header-name and converts underscores to hyphens; locate the
docstrings that contain the text "**headers: Additional x- headers. Keyword
underscores are converted to hyphens..." and update both occurrences (the one
around the shown diff and the other similar block) so the example and
explanation accurately reflect "x-" prefix behavior.
- Around line 91-94: The code that builds request_headers (using request_headers
= {"x-output-dtype": np.dtype(output_dtype).name} followed by
{f"x-{k.replace('_', '-')}": v for k, v in headers.items()}) should validate
custom header keys to avoid producing accidental "x-x-*" headers: before
constructing the prefixed header names, check each incoming key in headers and
raise a clear ValueError if key.lower().startswith("x") (covers "x_", "x-" and
"x...") so callers must not include the "x" prefix; apply the same validation
and behavior to the other identical block referenced around lines 190-193, and
keep the existing underscore-to-dash replacement for valid keys.
---
Nitpick comments:
In `@rationai/resources/models.py`:
- Around line 91-94: The header normalization logic that builds request_headers
(using np.dtype(output_dtype).name and mapping headers via f"x-{k.replace('_',
'-')"} is duplicated between the sync and async request methods; extract this
into a single helper function (e.g., build_request_headers or
normalize_request_headers) that takes output_dtype and headers and returns the
normalized dict, then replace the inline blocks in the methods that currently
assign request_headers with calls to that helper (ensure the helper imports
numpy as np or accepts already-normalized dtype string). This centralizes the
logic and removes duplication referenced by the request_headers usage in the
existing methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14e30c20-3c12-4a6a-93db-880df62cb623
📒 Files selected for processing (2)
docs/learn/how-it-works.mdrationai/resources/models.py
Summary by CodeRabbit
Documentation
New Features