[Platform][Voyage] Embedding update #359
Conversation
natewiebe13
requested review from
chr-hertel,
Nyholm and
OskarStark
as code owners
OskarStark
changed the title
natewiebe13
force-pushed
the
voyage-embedding-update
branch
2 times, most recently
from
e00eb5b to
8dddb8d
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for newer Voyage embedding models and additional configuration options. The changes introduce the newer V3.5 model family and CODE_3 model, while updating the default model to the more cost-effective V3_5_LITE.
- Added support for V3.5 model family (V3_5, V3_5_LITE) and V3_LARGE, CODE_3 models
- Enhanced model configuration with input_type, truncation, and dimensions options
- Updated default model from V3 to V3_5_LITE for better cost efficiency
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/platform/src/Bridge/Voyage/Voyage.php | Added new model constants, input type constants, enhanced constructor documentation, and changed default model |
| src/platform/src/Bridge/Voyage/ModelClient.php | Added support for new model options (input_type, truncation, output_dimension) in API requests |
| src/platform/tests/Bridge/Voyage/ResultConverterTest.php | Added test coverage for new model constants |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| */ | ||
| class Voyage extends Model | ||
| { | ||
| /** Supported dimensions: 2048, 1024, 512, or 256 */ |
There was a problem hiding this comment.
It's useful to have this information here. Could we validate the dimensions option in the constructor and indicate the dimensions supported in case of error?
There was a problem hiding this comment.
I'm usually a bit reluctant to bring in those validations because we don't really own that and don't rely on that ourselves, but it is only with Voyage.
Change scenario could be that they open up or change the support, the lib user want to leverage it, but is limited by our design - and we would need to patch and release to enable them.
We should check tho what happens if the user submits wrong options to the API, and if the error message gets back to the developer - that one of our weak spots still ...
There was a problem hiding this comment.
@natewiebe13 by any chance, did you check that already?
There was a problem hiding this comment.
@chr-hertel a \Symfony\AI\Platform\Exception\RuntimeException is thrown with Response does not contain embedding data.
| * @param array{dimensions?: int, input_type?: self::INPUT_TYPE_*, truncation?: bool} $options | ||
| */ | ||
| public function __construct(string $name = self::V3, array $options = []) | ||
| public function __construct(string $name = self::V3_5_LITE, array $options = []) |
There was a problem hiding this comment.
lets not change the default version
There was a problem hiding this comment.
I wonder if it might be best to not specify a default. People are notorious for using the defaults because it's easy. Keeping an outdated, more expensive model as a default likely isn't the best long-term. If not, what would the process be for updating the default? If the hesitation is going to a "lite" version, what about V3_5 as the new default?
OskarStark
force-pushed
the
voyage-embedding-update
branch
from
3e6c545 to
ac27544
Compare
|
Thank you, I made some changes while merging. |
@OskarStark the one thing I'll mention is that the comments regarding dimensions was copied from |
|
Let's remove them there as well I would say. We are not able to keep track of this, and if they may change the number of dimensions in the future. Is this the raw exception from the API? I think not and there is an open PR/issue to improve error logging from the platforms, so we should tackle this with better logging |
|
@OskarStark that's the content from the exception being thrown ( The raw response json is: {"detail":"Value '1234' supplied for argument 'output_dimension' is not valid -- accepted values for 'voyage-3.5-lite' are [256, 512, 1024, 2048]."} |
|
Thats super helpful, so lets propagate this raw error from the vendor somehow |
…ons (OskarStark) This PR was merged into the main branch. Discussion ---------- [Platform][Gemini][Embeddings] Remove comments for dimensions | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Docs? | no | Issues | Follows #359 | License | MIT cc `@natewiebe13` Commits ------- 3d07897 [Platform][Gemini][Embeddings] Remove comments for dimensions
Added additional options for working with Voyage embeddings. I did also bump the default model used, as the newer one is cheaper and higher quality (ref: https://blog.voyageai.com/2025/05/20/voyage-3-5/)