Uh oh!
There was an error while loading. Please reload this page.
Evaluation: Show cost - #113
Conversation
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 53 minutes and 13 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds per-run cost data: new cost types and formatting util, a cost icon component, and UI/tooltip in EvalRunCard showing total USD and a hoverable breakdown (response/embedding) with fixed-position tooltip and scroll-to-close behavior. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(main)/evaluations/[id]/page.tsx:
- Around line 755-757: Reformat the inline JSX conditional that renders token
counts to satisfy Prettier: break the expression onto multiple lines and align
parentheses/spacing around the conditional expression so it matches surrounding
JSX formatting (the block using job.cost.response.input_tokens and
job.cost.response.output_tokens and calling formatTokenCount). Update the
conditional rendering around that span (the JSX that checks
job.cost.response.input_tokens != null && job.cost.response.output_tokens !=
null) to follow Prettier’s preferred line breaks and indentation.
- Around line 698-726: The cost card currently only checks job.cost before
calling formatCostUSD(job.cost.total_cost_usd), which can throw if
total_cost_usd is missing; update the render guard in the Cost Breakdown block
(the JSX that renders Total Cost and calls formatCostUSD) to explicitly verify
total_cost_usd exists and is a valid number (e.g., job.cost?.total_cost_usd !=
null and Number.isFinite(job.cost.total_cost_usd)) before calling formatCostUSD,
and render a fallback (e.g., "—" or "N/A") when it's absent; adjust the
condition that wraps the Total Cost div (where formatCostUSD is invoked) so
formatCostUSD is only called when the value is present and valid.
In `@app/components/utils.ts`:
- Around line 75-80: The function formatCostUSD currently calls toFixed on
possibly invalid inputs and can produce "$NaN" or throw; modify formatCostUSD to
first validate the input (e.g., Number.isFinite(cost)) and if it's not a finite
number return a safe fallback like "$0.00"; only perform the cost < 0.01 branch
and call cost.toFixed(4) / toFixed(2) after the finite check so toFixed is never
invoked on bad values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9b1507a-6bd0-4b38-a642-0a1ec4400c2e
📒 Files selected for processing (4)
app/(main)/evaluations/[id]/page.tsxapp/components/evaluations/EvalRunCard.tsxapp/components/types.tsapp/components/utils.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/components/evaluations/EvalRunCard.tsx (1)
149-175: Unify tooltip width/position constants to avoid edge clipping drift.Line 149 uses
tooltipWidth = 280while Line 171 renderswidth: "260px". This can mis-clamp near the viewport edge. Consider sharing one constant and clamping top as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/evaluations/EvalRunCard.tsx` around lines 149 - 175, The tooltip width/position math is inconsistent: the onMouseEnter handler declares tooltipWidth = 280 but the rendered style uses width: "260px", which can cause clamping drift and edge clipping; update the component to use a single shared constant for tooltip width (replace the local tooltipWidth and the hard-coded "260px" with a single constant such as TOOLTIP_WIDTH) and use that same value when computing clampedLeft and when setting the style, and also clamp/set the tooltip top (using rect.top/rect.height and window.scrollY or Math.max/Math.min) before calling setCostTooltipPos so costTooltipPos, setCostTooltipPos, isCostTooltipOpen and the tooltip style remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/evaluations/EvalRunCard.tsx`:
- Around line 165-195: The tooltip can end up empty because it only renders
job.cost.response and job.cost.embedding; update the EvalRunCard tooltip
rendering to include a fallback row when neither response nor embedding exist:
inside the isCostTooltipOpen block (where costTooltipPos and formatCostUSD are
used) add a default line that displays a descriptive label like "Total cost" and
formats job.cost.total (or job.cost.cost_usd / computed total) so the tooltip
always shows at least the aggregate cost when EvalCost lacks response/embedding
entries.
- Around line 141-197: The tooltip trigger is a plain div with only mouse
handlers; make it keyboard and touch accessible by replacing or enhancing the
trigger element (the current inline div that references isCostTooltipOpen,
setIsCostTooltipOpen, costTooltipPos, setCostTooltipPos) so it is focusable and
operable: add role="button" or use a real <button>, include tabIndex,
aria-label/aria-describedby (pointing to the tooltip container) and
aria-expanded tied to isCostTooltipOpen, add onFocus/onBlur handlers that mirror
onMouseEnter/onMouseLeave to set costTooltipPos and open/close, implement
onKeyDown to open on Enter/Space and close on Escape, and add onClick to support
touch toggling; ensure the tooltip container is referenced by id for
accessibility and retains pointer-events: none only if you still need it
visually but allow screen readers to access its contents.
---
Nitpick comments:
In `@app/components/evaluations/EvalRunCard.tsx`:
- Around line 149-175: The tooltip width/position math is inconsistent: the
onMouseEnter handler declares tooltipWidth = 280 but the rendered style uses
width: "260px", which can cause clamping drift and edge clipping; update the
component to use a single shared constant for tooltip width (replace the local
tooltipWidth and the hard-coded "260px" with a single constant such as
TOOLTIP_WIDTH) and use that same value when computing clampedLeft and when
setting the style, and also clamp/set the tooltip top (using
rect.top/rect.height and window.scrollY or Math.max/Math.min) before calling
setCostTooltipPos so costTooltipPos, setCostTooltipPos, isCostTooltipOpen and
the tooltip style remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b81330e1-c480-433c-b122-4a853d4798ca
📒 Files selected for processing (2)
app/components/evaluations/EvalRunCard.tsxapp/components/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/utils.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| <svg | ||
| className="w-3.5 h-3.5 flex-shrink-0" | ||
| fill="none" | ||
| viewBox="0 0 24 24" | ||
| stroke="currentColor" | ||
| strokeWidth={2} | ||
| > | ||
| <path | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| d="M12 6v12m-3-2.818.879.659c1.171.879 3.07.879 4.242 0 1.172-.879 1.172-2.303 0-3.182C13.536 12.219 12.768 12 12 12c-.725 0-1.45-.22-2.003-.659-1.106-.879-1.106-2.303 0-3.182s2.9-.879 4.006 0l.415.33M21 12a9 9 0 1 1-18 0 9 9 0 0 1 18 0Z" | ||
| /> | ||
| </svg> |
There was a problem hiding this comment.
Move this icon inside the components/icons corresponding module and then import that icon here, and if the icon is already exist then directly import and use.
| className="inline-flex items-center justify-center w-4 h-4 rounded-full text-xs font-normal cursor-help" | ||
| style={{ | ||
| backgroundColor: isCostTooltipOpen ? "#171717" : "#fafafa", | ||
| color: isCostTooltipOpen ? "#ffffff" : "#737373", | ||
| }} |
There was a problem hiding this comment.
| className="inline-flex items-center justify-center w-4 h-4 rounded-full text-xs font-normal cursor-help" | |
| style={{ | |
| backgroundColor: isCostTooltipOpen ? "#171717" : "#fafafa", | |
| color: isCostTooltipOpen ? "#ffffff" : "#737373", | |
| }} | |
| className={`inline-flex items-center justify-center w-4 h-4 rounded-full text-xs font-normal cursor-help ${isCostTooltipOpen ? "bg-[#171717] text-white" : "text-[#737373]"}`} |
| > | ||
| {job.cost.response && ( | ||
| <div className="flex justify-between gap-3"> | ||
| <span style={{ color: "#a3a3a3" }}> |
There was a problem hiding this comment.
| <spanstyle={{color: "#a3a3a3"}}> | |
| <spanclassName="text-[#a3a3a3]"> |
| )} | ||
| {job.cost.embedding && ( | ||
| <div className="flex justify-between gap-3"> | ||
| <span style={{ color: "#a3a3a3" }}> |
There was a problem hiding this comment.
| <spanstyle={{color: "#a3a3a3"}}> | |
| <spanclassName="text-[#a3a3a3]"> |
| className="fixed z-50 px-3 py-2 rounded-md text-xs whitespace-normal pointer-events-none space-y-1" | ||
| style={{ | ||
| backgroundColor: "#171717", | ||
| color: "#ffffff", | ||
| width: "260px", | ||
| boxShadow: "0 4px 6px rgba(0, 0, 0, 0.1)", | ||
| top: costTooltipPos.top, | ||
| left: costTooltipPos.left, | ||
| transform: "translateY(-100%)", | ||
| }} |
There was a problem hiding this comment.
| className="fixed z-50 px-3 py-2 rounded-md text-xs whitespace-normal pointer-events-none space-y-1" | |
| style={{ | |
| backgroundColor: "#171717", | |
| color: "#ffffff", | |
| width: "260px", | |
| boxShadow: "0 4px 6px rgba(0, 0, 0, 0.1)", | |
| top: costTooltipPos.top, | |
| left: costTooltipPos.left, | |
| transform: "translateY(-100%)", | |
| }} | |
| className="fixed z-50 px-3 py-2 rounded-md text-xs whitespace-normal pointer-events-none space-y-1 bg-[#171717] text-white w-[260px] shadow-md -translate-y-full" | |
| style={{ | |
| top: costTooltipPos.top, | |
| left: costTooltipPos.left, | |
| }} |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/evaluations/EvalRunCard.tsx`:
- Around line 132-145: The tooltip currently always positions above the card
(top: rect.top - 8) which can make it overflow/clipped when the card is near the
top; update the onMouseEnter handler that sets setCostTooltipPos and
setIsCostTooltipOpen to detect available space and flip the tooltip below when
needed: compute an estimated tooltipHeight (or use a constant like 40–56px), and
if rect.top < tooltipHeight + 8 set top = rect.bottom + 8 and mark the tooltip
to render without the -translate-y-full transform (or set a flag to choose the
below-placement class); otherwise keep top = rect.top - 8 and use the
above-placement transform. Also ensure left clamping logic using tooltipWidth
remains unchanged and consider window.innerHeight bounds when flipping.
- Around line 127-178: The DOM nesting is invalid because the outer element
using className "flex items-center gap-1.5" (currently a <span>) contains <div>
children for the tooltip trigger and body; update the wrapper in EvalRunCard
(the element that wraps CostIcon, formatCostUSD, the tooltip trigger that calls
setCostTooltipPos/setIsCostTooltipOpen, and the conditional tooltip that reads
costTooltipPos and job.cost) to be a <div> (or alternatively change the inner
tooltip <div>s to <span>s) so the structure is semantically valid and React's
validateDOMNesting warnings are resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fa487037-2fad-40bc-b5e1-bdf76e485976
📒 Files selected for processing (2)
app/components/evaluations/EvalRunCard.tsxapp/components/icons/evaluations/CostIcon.tsx
| <span className="flex items-center gap-1.5"> | ||
| <CostIcon className="flex-shrink-0" /> | ||
| {formatCostUSD(job.cost.total_cost_usd)} | ||
| <div | ||
| className={`inline-flex items-center justify-center w-4 h-4 rounded-full text-xs font-normal cursor-help ${isCostTooltipOpen ? "bg-[#171717] text-white" : "text-[#737373]"}`} | ||
| onMouseEnter={(e) => { | ||
| const rect = e.currentTarget.getBoundingClientRect(); | ||
| const tooltipWidth = 280; | ||
| const centerX = rect.left + rect.width / 2; | ||
| const clampedLeft = Math.min( | ||
| Math.max(centerX - tooltipWidth / 2, 8), | ||
| window.innerWidth - tooltipWidth - 8, | ||
| ); | ||
| setCostTooltipPos({ | ||
| top: rect.top - 8, | ||
| left: clampedLeft, | ||
| }); | ||
| setIsCostTooltipOpen(true); | ||
| }} | ||
| onMouseLeave={() => setIsCostTooltipOpen(false)} | ||
| > | ||
| i | ||
| </div> | ||
| {isCostTooltipOpen && ( | ||
| <div | ||
| className="fixed z-50 px-3 py-2 rounded-md text-xs whitespace-normal pointer-events-none space-y-1 bg-[#171717] text-white w-[260px] shadow-md -translate-y-full" | ||
| style={{ | ||
| top: costTooltipPos.top, | ||
| left: costTooltipPos.left, | ||
| }} | ||
| > | ||
| {job.cost.response && ( | ||
| <div className="flex justify-between gap-3"> | ||
| <span className="text-[#a3a3a3]"> | ||
| Response generation | ||
| </span> | ||
| <span>{formatCostUSD(job.cost.response.cost_usd)}</span> | ||
| </div> | ||
| )} | ||
| {job.cost.embedding && ( | ||
| <div className="flex justify-between gap-3"> | ||
| <span className="text-[#a3a3a3]"> | ||
| Cosine similarity calculation | ||
| </span> | ||
| <span> | ||
| {formatCostUSD(job.cost.embedding.cost_usd)} | ||
| </span> | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} | ||
| </span> |
There was a problem hiding this comment.
Invalid DOM nesting: <div> elements inside <span>.
The outer wrapper on line 127 is a <span> (phrasing content), but it now contains <div> children (the tooltip trigger on line 130 and the tooltip body on line 151). React will emit a validateDOMNesting warning and some browsers may reflow unexpectedly. Change the wrapper to a <div> (or make the trigger/tooltip <span>s).
Proposed fix
- <span className="flex items-center gap-1.5">+ <div className="flex items-center gap-1.5 relative">
<CostIcon className="flex-shrink-0" />
{formatCostUSD(job.cost.total_cost_usd)}
...
- </span>+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <spanclassName="flex items-center gap-1.5"> | |
| <CostIconclassName="flex-shrink-0"/> | |
| {formatCostUSD(job.cost.total_cost_usd)} | |
| <div | |
| className={`inline-flex items-center justify-center w-4 h-4 rounded-full text-xs font-normal cursor-help ${isCostTooltipOpen ? "bg-[#171717] text-white" : "text-[#737373]"}`} | |
| onMouseEnter={(e)=>{ | |
| constrect=e.currentTarget.getBoundingClientRect(); | |
| consttooltipWidth=280; | |
| constcenterX=rect.left+rect.width/2; | |
| constclampedLeft=Math.min( | |
| Math.max(centerX-tooltipWidth/2,8), | |
| window.innerWidth-tooltipWidth-8, | |
| ); | |
| setCostTooltipPos({ | |
| top: rect.top-8, | |
| left: clampedLeft, | |
| }); | |
| setIsCostTooltipOpen(true); | |
| }} | |
| onMouseLeave={()=>setIsCostTooltipOpen(false)} | |
| > | |
| i | |
| </div> | |
| {isCostTooltipOpen&&( | |
| <div | |
| className="fixed z-50 px-3 py-2 rounded-md text-xs whitespace-normal pointer-events-none space-y-1 bg-[#171717] text-white w-[260px] shadow-md -translate-y-full" | |
| style={{ | |
| top: costTooltipPos.top, | |
| left: costTooltipPos.left, | |
| }} | |
| > | |
| {job.cost.response&&( | |
| <divclassName="flex justify-between gap-3"> | |
| <spanclassName="text-[#a3a3a3]"> | |
| Responsegeneration | |
| </span> | |
| <span>{formatCostUSD(job.cost.response.cost_usd)}</span> | |
| </div> | |
| )} | |
| {job.cost.embedding&&( | |
| <divclassName="flex justify-between gap-3"> | |
| <spanclassName="text-[#a3a3a3]"> | |
| Cosinesimilaritycalculation | |
| </span> | |
| <span> | |
| {formatCostUSD(job.cost.embedding.cost_usd)} | |
| </span> | |
| </div> | |
| )} | |
| </div> | |
| )} | |
| </span> | |
| <divclassName="flex items-center gap-1.5 relative"> | |
| <CostIconclassName="flex-shrink-0"/> | |
| {formatCostUSD(job.cost.total_cost_usd)} | |
| <div | |
| className={`inline-flex items-center justify-center w-4 h-4 rounded-full text-xs font-normal cursor-help ${isCostTooltipOpen ? "bg-[`#171717`] text-white" : "text-[`#737373`]"}`} | |
| onMouseEnter={(e)=>{ | |
| constrect=e.currentTarget.getBoundingClientRect(); | |
| consttooltipWidth=280; | |
| constcenterX=rect.left+rect.width/2; | |
| constclampedLeft=Math.min( | |
| Math.max(centerX-tooltipWidth/2,8), | |
| window.innerWidth-tooltipWidth-8, | |
| ); | |
| setCostTooltipPos({ | |
| top: rect.top-8, | |
| left: clampedLeft, | |
| }); | |
| setIsCostTooltipOpen(true); | |
| }} | |
| onMouseLeave={()=>setIsCostTooltipOpen(false)} | |
| > | |
| i | |
| </div> | |
| {isCostTooltipOpen&&( | |
| <div | |
| className="fixed z-50 px-3 py-2 rounded-md text-xs whitespace-normal pointer-events-none space-y-1 bg-[`#171717`] text-white w-[260px] shadow-md -translate-y-full" | |
| style={{ | |
| top: costTooltipPos.top, | |
| left: costTooltipPos.left, | |
| }} | |
| > | |
| {job.cost.response&&( | |
| <divclassName="flex justify-between gap-3"> | |
| <spanclassName="text-[`#a3a3a3`]"> | |
| Responsegeneration | |
| </span> | |
| <span>{formatCostUSD(job.cost.response.cost_usd)}</span> | |
| </div> | |
| )} | |
| {job.cost.embedding&&( | |
| <divclassName="flex justify-between gap-3"> | |
| <spanclassName="text-[`#a3a3a3`]"> | |
| Cosinesimilaritycalculation | |
| </span> | |
| <span> | |
| {formatCostUSD(job.cost.embedding.cost_usd)} | |
| </span> | |
| </div> | |
| )} | |
| </div> | |
| )} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/evaluations/EvalRunCard.tsx` around lines 127 - 178, The DOM
nesting is invalid because the outer element using className "flex items-center
gap-1.5" (currently a <span>) contains <div> children for the tooltip trigger
and body; update the wrapper in EvalRunCard (the element that wraps CostIcon,
formatCostUSD, the tooltip trigger that calls
setCostTooltipPos/setIsCostTooltipOpen, and the conditional tooltip that reads
costTooltipPos and job.cost) to be a <div> (or alternatively change the inner
tooltip <div>s to <span>s) so the structure is semantically valid and React's
validateDOMNesting warnings are resolved.
| onMouseEnter={(e) => { | ||
| const rect = e.currentTarget.getBoundingClientRect(); | ||
| const tooltipWidth = 280; | ||
| const centerX = rect.left + rect.width / 2; | ||
| const clampedLeft = Math.min( | ||
| Math.max(centerX - tooltipWidth / 2, 8), | ||
| window.innerWidth - tooltipWidth - 8, | ||
| ); | ||
| setCostTooltipPos({ | ||
| top: rect.top - 8, | ||
| left: clampedLeft, | ||
| }); | ||
| setIsCostTooltipOpen(true); | ||
| }} |
There was a problem hiding this comment.
Tooltip can overflow the top of the viewport.
Horizontal position is clamped, but vertical is rect.top - 8 combined with -translate-y-full — when the card is near the top of the viewport the tooltip renders above the page (clipped/unreachable). Consider flipping below (rect.bottom + 8 without the translate) when rect.top is less than estimated tooltip height.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/evaluations/EvalRunCard.tsx` around lines 132 - 145, The
tooltip currently always positions above the card (top: rect.top - 8) which can
make it overflow/clipped when the card is near the top; update the onMouseEnter
handler that sets setCostTooltipPos and setIsCostTooltipOpen to detect available
space and flip the tooltip below when needed: compute an estimated tooltipHeight
(or use a constant like 40–56px), and if rect.top < tooltipHeight + 8 set top =
rect.bottom + 8 and mark the tooltip to render without the -translate-y-full
transform (or set a flag to choose the below-placement class); otherwise keep
top = rect.top - 8 and use the above-placement transform. Also ensure left
clamping logic using tooltipWidth remains unchanged and consider
window.innerHeight bounds when flipping.
Summary
Target issue is #39
New Features
Summary by CodeRabbit