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
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
| <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.
| <span style={{ color: "#a3a3a3" }}> | |
| <span className="text-[#a3a3a3]"> |
| )} | ||
| {job.cost.embedding && ( | ||
| <div className="flex justify-between gap-3"> | ||
| <span style={{ color: "#a3a3a3" }}> |
There was a problem hiding this comment.
| <span style={{ color: "#a3a3a3" }}> | |
| <span className="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.
| <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> | |
| <div className="flex items-center gap-1.5 relative"> | |
| <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> | |
| )} | |
| </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