-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat: show dash instead of zero for missing data on evals page #7748
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
|
🚀 Preview deployed! Your changes have been deployed to Vercel: Preview URL: https://roo-code-website-pjaimjzgd-roo-code.vercel.app This preview will be updated automatically when you push new commits to this PR. |
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.
Reviewing my own code is like debugging in production - technically possible but morally questionable.
|
|
||
| export const formatCurrency = (amount: number) => formatter.format(amount) | ||
| export const formatCurrency = (amount: number | null | undefined) => { | ||
| if (amount === null || amount === undefined) { |
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.
Would it be good to add unit tests for this new null/undefined handling behavior? I notice there aren't any tests for these formatting functions yet, and it would help ensure the "-" fallback works correctly across all edge cases.
| export const formatDuration = (durationMs: number) => { | ||
| export const formatDuration = (durationMs: number | null | undefined) => { | ||
| if (durationMs === null || durationMs === undefined) { | ||
| return "-" |
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.
Same null-checking pattern as the other formatters. Could we consider extracting this into a shared utility like formatWithFallback(value, formatter) to reduce duplication? Though I understand if we prefer the explicit approach for clarity.
| <TableCell className="border-r">{formatCurrency(run.taskMetrics.cost)}</TableCell> | ||
| <TableCell className="text-muted-foreground"> | ||
| {formatScore(run.languageScores?.go ?? 0)}% | ||
| {run.languageScores?.go !== undefined ? `${formatScore(run.languageScores.go)}%` : "-"} |
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 that we're distinguishing between genuine zero values and missing data? For example, a model with 0% score will show "0%" while missing scores show "-". Just confirming this is the desired UX behavior.
| export const formatTokens = (tokens: number, decimals = 0) => { | ||
| export const formatTokens = (tokens: number | null | undefined, decimals = 0) => { | ||
| if (tokens === null || tokens === undefined) { | ||
| return "-" |
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 chart rendering code in evals.tsx still passes potentially null/undefined values to formatCurrency within the chart components. Is the Recharts library handling these gracefully, or should we add null checks before passing to the chart components?
| ...run, | ||
| label: run.description || run.model, | ||
| score: formatScore(run.passed / (run.passed + run.failed)), | ||
| score: Math.round((run.passed / (run.passed + run.failed)) * 100), |
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 guarding against a zero or missing denominator. If run.passed + run.failed is 0 (or if either is undefined), Math.round(NaN) will return NaN rather than '-' for missing data. Add a check to return an appropriate fallback (e.g. undefined or '-' value) when data is missing.
| score: Math.round((run.passed / (run.passed + run.failed)) * 100), | |
| score: (run.passed != null && run.failed != null && (run.passed + run.failed) > 0) ? Math.round((run.passed / (run.passed + run.failed)) * 100) : "-", |
This PR updates the evals page to display "-" instead of "0" for missing data, providing better visual clarity when data is not available.
Changes
formatCurrency,formatDuration,formatTokens) to handle null/undefined values and return "-"Context
This change was requested via Slack to improve the user experience by clearly distinguishing between actual zero values and missing data on the evals page.
Fixes: Display "-" instead of "0" for missing data on evals page
Important
Display "-" instead of "0" for missing data on the evals page by updating formatting functions and evals component.
evals.tsx: Passundefinedfor missing model info and language scores.formatCurrency,formatDuration,formatTokens: Return "-" fornullorundefinedvalues.This description was created by
for be1ef46. You can customize this summary. It will automatically update as commits are pushed.