Skip to content

Conversation

@mrubens
Copy link
Collaborator

@mrubens mrubens commented Nov 21, 2025

Ideally we can share more between providers - I think this works for Groq and Baseten at least.


Important

Enhance base OpenAI-compatible provider for better compatibility and flexibility across different providers, with updates to test assertions and usage metrics handling.

  • Behavior:
    • Refactor test assertions to use toMatchObject for partial matching in base-openai-compatible-provider.spec.ts, featherless.spec.ts, fireworks.spec.ts, groq.spec.ts, io-intelligence.spec.ts, sambanova.spec.ts, and zai.spec.ts.
    • Update createMessage and completePrompt methods in base-openai-compatible-provider.ts to handle reasoning content and usage metrics more flexibly.
  • Models:
    • Add processUsageMetrics method in base-openai-compatible-provider.ts to calculate and yield usage metrics.
    • Modify createStream and createMessage methods to include reasoning parameters if supported by the model.
  • Misc:
    • Remove redundant code in groq.ts and zai.ts by leveraging the base provider's functionality.
    • Ensure cacheWriteTokens and cacheReadTokens are undefined when zero in groq.spec.ts and zai.spec.ts.

This description was created by Ellipsis for 3b468dd. You can customize this summary. It will automatically update as commits are pushed.

@mrubens mrubens requested review from cte and jr as code owners November 21, 2025 06:02
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Nov 21, 2025
@roomote
Copy link
Contributor

roomote bot commented Nov 21, 2025

Rooviewer Clock   See task on Roo Cloud

Re-reviewed commit 3b468dd. ZAI refactoring looks good, but the previously flagged issue remains unaddressed.

  • Inconsistent cost calculation between FeatherlessHandler's DeepSeek-R1 path and other models
Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

cacheWriteTokens: cacheWriteTokens || undefined,
cacheReadTokens: cacheReadTokens || undefined,
totalCost,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new processUsageMetrics method in the base class now includes cost calculation via calculateApiCostOpenAI. However, FeatherlessHandler has a custom createMessage implementation for DeepSeek-R1 models that yields usage without cost calculation (lines 79-85). This creates an inconsistency: non-R1 models (which use super.createMessage()) will get cost calculation, but R1 models won't. Consider updating the R1 path to also use processUsageMetrics or call calculateApiCostOpenAI directly to maintain consistent cost tracking across all Featherless models.

Fix it with Roo Code or mention @roomote and request a fix.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Nov 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

3 participants