-
Notifications
You must be signed in to change notification settings - Fork 434
[feat]: show provider model input/output cost in playground #3326
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
[feat]: show provider model input/output cost in playground #3326
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
mmabrouk
left a comment
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.
Thanks @ashrafchowdury for the PR
I have cleaned up the Python code.
For the rest I am not the best judge, @ardaerzin can judge better. But the AI mentioned issues that seems very reasonable.
5. metadata.ts ⚠️ Minor Issues
Good parts:
findModelMetadata()function is well-documented- Backward compatibility check is smart
Issues:
| Issue | Severity | Details |
|---|---|---|
| Nested loop on every option | Low | findModelMetadata() iterates through all providers for each model lookup. Could be slow with many models. Consider flattening the lookup once at initialization. |
| Type assertion | Low | as Record<string, Record<string, unknown>> - slightly loose typing |
Suggested improvement:
// Build a flat lookup map once instead of nested search each time
function buildModelMetadataLookup(
metadata?: Record<string, unknown>
): Map<string, Record<string, unknown>> {
const lookup = new Map<string, Record<string, unknown>>()
if (!metadata) return lookup
for (const providerData of Object.values(metadata)) {
if (providerData && typeof providerData === "object") {
for (const [model, modelData] of Object.entries(providerData)) {
lookup.set(model, modelData as Record<string, unknown>)
}
}
}
return lookup
}6. SelectLLMProvider/index.tsx ⚠️ Needs Attention
This is the main UI component and has several issues:
Issues
| Issue | Severity | Details |
|---|---|---|
| Popover inside Option | Medium | Wrapping content in <Popover> inside a Select option can cause interaction issues (clicks, hover states). |
onClick={(e) => e.stopPropagation()} |
Medium | This is a workaround that suggests the approach is fighting against Ant Design's Select behavior. |
Hardcoded $ currency |
Low | ${metadata.input} / 1M - might want to format numbers properly (e.g., $0.50 vs $0.5). |
| Invalid Tailwind class | Low | "w-42" isn't a standard Tailwind class. Should be w-[168px] or similar. |
| Duplicate render logic | Low | renderOption and renderOptionContent are called in multiple places. |
Recommended Fix
Replace Popover with Tooltip for a cleaner, lighter-weight solution:
import {Tooltip} from "antd"
const renderOption = (option: ProviderOption) => {
const content = renderOptionContent(option)
if (option.metadata) {
return (
<Tooltip
title={renderTooltipContent(option.metadata)}
placement="right"
mouseEnterDelay={0.3}
>
{content}
</Tooltip>
)
}
return content
}This removes the need for stopPropagation() and works better with Ant Design's Select component.
Number Formatting
Consider formatting the cost values:
const formatCost = (cost: number) => {
return cost < 0.01 ? cost.toFixed(4) : cost.toFixed(2)
}
// Usage
<Typography.Text className="text-[10px] text-nowrap">
${formatCost(metadata.input)} / 1M
</Typography.Text>Priority Actions
- High: Fix
SelectLLMProvider- replacePopoverwithTooltipto avoid interaction issues - Medium: Fix invalid Tailwind class
w-42 - Low: Add number formatting for cost display
- Low: Optimize
findModelMetadata()lookup inmetadata.ts
It's yours and @ardaerzin to judge the feedback. From my side I agree with the number formatting feedback from a product perspective
|
Alright, thank you @mmabrouk 🙏 |
What's new??
Added input/output cost for model usage in the playground when selecting a model. We are fetching the model cost data from litellm in the backend and passing it to the frontend.
Note: We don't get input/output cost for all the models listed out on the playground, but we do get it for most of them.
Click to see preview
QA