Skip to content

fix: prevent negative cost values and improve label visibility in evals chart #7830

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 9, 2025
Merged

fix: prevent negative cost values and improve label visibility in evals chart #7830

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/web-roo-code/src/app/evals/plot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,13 @@ export const Plot = ({ tableData }: PlotProps) => {
<>
<div className="pt-4 pb-8 font-mono">Cost x Score</div>
<ChartContainer config={chartConfig} className="h-[500px] w-full">
<ScatterChart margin={{ top: 0, right: 0, bottom: 0, left: 20 }}>
<ScatterChart margin={{ top: 20, right: 0, bottom: 0, left: 20 }}>
<XAxis
type="number"
dataKey="cost"
name="Cost"
domain={[
(dataMin: number) => Math.round((dataMin - 5) / 5) * 5,
(dataMin: number) => Math.max(0, Math.round((dataMin - 5) / 5) * 5),
Copy link
Author

@roomote roomote bot Sep 9, 2025

Choose a reason for hiding this comment

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

Good fix for preventing negative costs! I notice the Y-axis already uses this same pattern on line 194, so this improves consistency.

One thought: could we also consider adding a small right margin (maybe 10-20px) to prevent potential label truncation on the right edge, similar to what we did for the top margin?

(dataMax: number) => Math.round((dataMax + 5) / 5) * 5,
Copy link
Author

@roomote roomote bot Sep 9, 2025

Choose a reason for hiding this comment

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

The value 5 appears to be used as a rounding factor here and in other domain calculations. Would it make sense to extract this to a named constant like AXIS_ROUNDING_FACTOR or add a comment explaining why 5 was chosen? This would make the intent clearer for future maintenance.

]}
tickFormatter={(value) => formatCurrency(value)}
Expand Down
Loading