-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Add Weights & Biases as a provider #7886
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've reviewed the changes and have some suggestions for improvement. Overall, the implementation follows existing patterns well and includes comprehensive i18n support. The code is clean and well-structured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the 20% maxTokens calculation intentional for all W&B models? I noticed this differs from other providers like OpenRouter which use varying percentages. Could we document why this specific ratio was chosen, or should it vary per model?
src/api/providers/wandb.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider at least logging these parsing errors for debugging purposes instead of silently ignoring them:
src/api/providers/wandb.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These token estimates seem quite rough (4 chars per token). Could we use a more accurate tokenizer, or at least document why these specific ratios were chosen? This could impact cost calculations significantly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test coverage could be improved. Consider adding tests for:
- Thinking token stripping functionality ()
- XmlMatcher integration for reasoning models
- Image content handling in messages
- Actual streaming response parsing with realistic data
These are critical features that should have test coverage.
src/api/providers/wandb.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The temperature is clamped to 0-2 range with a comment "W&B typically supports 0-2 range". Should this be configurable per model or documented in the model definitions? Different models might have different optimal ranges.
src/api/providers/wandb.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, should this also use the localized error format like in ?
src/api/providers/wandb.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding JSDoc comments to the main class and its public methods. This would improve maintainability and help other developers understand the API better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? I think these models have documented maxTokens.
Can you change these so that it matches the actual value of the models?
- Add wandb provider implementation with OpenAI-compatible API - Add wandb models with full context window support (maxTokens = contextWindow) - Add comprehensive error handling with localized messages - Add thinking token support for reasoning models - Add wandb API key settings to all language locales - Add wandb error translations to all supported languages - Support for multimodal models and reasoning effort - Proper token usage tracking and cost calculation
862f039 to
df50f89
Compare
|
Is the failing unit test legit? |
Unrelated to my changes i don't know why this keeps happening |
|
We are not adding this provider at this time. |
Important
Add Weights & Biases as a provider with full API support, error handling, and validation updates.
wandbprovider with OpenAI-compatible API inwandb.ts.wandbApiKeytoglobal-settings.tsandprovider-settings.ts.ApiOptions.tsxandWandb.tsxfor API key input and management.wandbin all supported languages inlocales.wandb.ts.validate.tsto includewandbAPI key validation.WandbHandlerinwandb.spec.ts.This description was created by
for 862f0390b070a926fc5899ba7847a729af6db303. You can customize this summary. It will automatically update as commits are pushed.