Refine website landing and sandbox copy#445
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds GPT token-counting tooling, a shared tool-call-comparison module, validation scripts that compute/verify GPT‑5 (o200k_base) token counts, centralizes sandbox tool-call logic, and introduces a resizable, accessible 3‑pane sandbox UI plus landing/CSS/content tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Script as compute-llm-call-tokens
participant Lib as tool-call-comparison
participant Tokenizer as gpt-tokenizer
participant Baseline as LLM_CALL_TOKENS
Script->>Lib: buildLlmRequest(flowKey, index)
Script->>Lib: buildFunctionCallOutput(flowKey, index)
Script->>Tokenizer: encode request/output -> token arrays
Tokenizer-->>Script: tokens per part (in/out)
Script->>Script: aggregate computed[flowKey] per-step totals
Script->>Baseline: compare computed vs exported baselines
alt mismatch
Script->>Script: exit non-zero (stale)
else match
Script-->>Script: print per-flow step totals
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Review rate limit: 0/5 reviews remaining, refill in 2 minutes and 13 seconds. Comment |
Benchmark Results392 benchmarks Interpreted: 🟢 48 improved · 🔴 97 regressed · 247 unchanged · avg +0.3% arraybuffer.js — Interp: 14 unch. · avg -1.3% · Bytecode: 🟢 1, 🔴 12, 1 unch. · avg -5.5%
arrays.js — Interp: 🔴 5, 14 unch. · avg -2.1% · Bytecode: 🔴 17, 2 unch. · avg -9.6%
async-await.js — Interp: 🔴 2, 4 unch. · avg -1.2% · Bytecode: 🔴 6 · avg -10.6%
async-generators.js — Interp: 2 unch. · avg -2.6% · Bytecode: 🔴 1, 1 unch. · avg -8.9%
base64.js — Interp: 🔴 2, 8 unch. · avg -1.3% · Bytecode: 🔴 10 · avg -8.9%
classes.js — Interp: 🔴 17, 14 unch. · avg -3.6% · Bytecode: 🟢 1, 🔴 13, 17 unch. · avg -2.9%
closures.js — Interp: 🔴 1, 10 unch. · avg -2.3% · Bytecode: 🔴 11 · avg -9.6%
collections.js — Interp: 🔴 1, 11 unch. · avg -0.7% · Bytecode: 🔴 12 · avg -9.8%
csv.js — Interp: 🔴 2, 11 unch. · avg -2.1% · Bytecode: 🔴 11, 2 unch. · avg -6.8%
destructuring.js — Interp: 🔴 6, 16 unch. · avg -2.1% · Bytecode: 🔴 21, 1 unch. · avg -8.8%
fibonacci.js — Interp: 🟢 1, 7 unch. · avg +0.7% · Bytecode: 🔴 8 · avg -10.5%
float16array.js — Interp: 🟢 11, 🔴 3, 18 unch. · avg +10.4% · Bytecode: 🟢 1, 🔴 29, 2 unch. · avg -8.9%
for-of.js — Interp: 🟢 7 · avg +6.7% · Bytecode: 🔴 7 · avg -9.3%
generators.js — Interp: 🔴 4 · avg -6.0% · Bytecode: 🔴 4 · avg -5.6%
iterators.js — Interp: 🔴 14, 28 unch. · avg -2.8% · Bytecode: 🟢 1, 🔴 29, 12 unch. · avg -5.5%
json.js — Interp: 🔴 13, 7 unch. · avg -4.0% · Bytecode: 🔴 19, 1 unch. · avg -8.9%
jsx.jsx — Interp: 🟢 16, 5 unch. · avg +5.2% · Bytecode: 🔴 17, 4 unch. · avg -6.8%
modules.js — Interp: 🔴 3, 6 unch. · avg -2.2% · Bytecode: 🔴 9 · avg -13.6%
numbers.js — Interp: 🔴 3, 8 unch. · avg -2.3% · Bytecode: 🔴 11 · avg -7.7%
objects.js — Interp: 🟢 3, 4 unch. · avg +3.9% · Bytecode: 🔴 6, 1 unch. · avg -9.3%
promises.js — Interp: 🔴 7, 5 unch. · avg -3.4% · Bytecode: 🔴 9, 3 unch. · avg -5.0%
regexp.js — Interp: 🔴 2, 9 unch. · avg -2.3% · Bytecode: 🔴 11 · avg -6.6%
strings.js — Interp: 🟢 6, 13 unch. · avg +2.9% · Bytecode: 🔴 18, 1 unch. · avg -7.4%
tsv.js — Interp: 9 unch. · avg +0.8% · Bytecode: 🔴 9 · avg -7.5%
typed-arrays.js — Interp: 🔴 11, 11 unch. · avg -2.1% · Bytecode: 🟢 6, 🔴 14, 2 unch. · avg -1.8%
uint8array-encoding.js — Interp: 🟢 4, 🔴 1, 13 unch. · avg +12.0% · Bytecode: 🟢 6, 🔴 9, 3 unch. · avg -0.9%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
Suite Timing
Measured on ubuntu-latest x64. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
website/src/components/sandbox.tsx (1)
841-865: Make the new pane resizers operable without a pointer.These handles are focusable buttons, but they only respond to drag events. Keyboard users can land on them and then get stuck with no way to resize the panes. Adding separator semantics plus ArrowLeft/ArrowRight handling would make the feature usable beyond pointer input.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/sandbox.tsx` around lines 841 - 865, The new resizer buttons (class "sb-resizer", aria-labels "Resize script and globals panes" and "Resize globals and result panes") are focusable but only handle pointer drags via startPaneResize; add keyboard support and separator semantics so non-pointer users can resize. Give the controls role="separator" and aria-orientation="horizontal" (and appropriate aria-valuenow/aria-valuemin/aria-valuemax if pane sizes are tracked), and add an onKeyDown handler that listens for ArrowLeft/ArrowRight (and optionally ArrowUp/ArrowDown) to call the same resize logic (e.g., invoke startPaneResize with a small step or call the internal resize function) and handle Home/End for extremes; keep existing onPointerDown behavior intact so pointer and keyboard both work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/src/app/globals.css`:
- Around line 2148-2155: The CSS selector .sb-field + .sb-field no longer
matches because .sb-resizer elements are inserted between fields; update the
rules under .sb-demo-body to target the field that follows a resizer or another
field by replacing .sb-field + .sb-field with both .sb-field + .sb-resizer +
.sb-field and .sb-resizer + .sb-field (and mirror this change inside the `@media`
(min-width: 900px) / 3-column override blocks referenced around the other
occurrence), so separators (border-top / border-left) still apply when
.sb-resizer sits between .sb-field elements.
- Around line 1116-1119: The rule limiting .container > .section-head and
.container > .mt-12 > .section-head with max-width: 80vw causes narrower text
blocks on small screens; remove or replace that declaration so these selectors
no longer constrain width (e.g., delete the max-width: 80vw line or set
max-width: none/100%) to allow the section-heads to fill the container naturally
and avoid excessive wrapping.
---
Nitpick comments:
In `@website/src/components/sandbox.tsx`:
- Around line 841-865: The new resizer buttons (class "sb-resizer", aria-labels
"Resize script and globals panes" and "Resize globals and result panes") are
focusable but only handle pointer drags via startPaneResize; add keyboard
support and separator semantics so non-pointer users can resize. Give the
controls role="separator" and aria-orientation="horizontal" (and appropriate
aria-valuenow/aria-valuemin/aria-valuemax if pane sizes are tracked), and add an
onKeyDown handler that listens for ArrowLeft/ArrowRight (and optionally
ArrowUp/ArrowDown) to call the same resize logic (e.g., invoke startPaneResize
with a small step or call the internal resize function) and handle Home/End for
extremes; keep existing onPointerDown behavior intact so pointer and keyboard
both work.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07223070-7b35-4ace-9c0e-30ef7563bb45
⛔ Files ignored due to path filters (1)
website/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
website/package.jsonwebsite/scripts/compute-llm-call-tokens.mjswebsite/src/app/globals.csswebsite/src/components/animated-output.tsxwebsite/src/components/landing.tsxwebsite/src/components/sandbox.tsxwebsite/src/lib/landing-data.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/src/components/sandbox.tsx (1)
493-532: Consider cleaning up event listeners on unmount.The
pointermoveandpointeruplisteners attached towindowwon't be cleaned up if the component unmounts mid-drag. This is unlikely in practice but could cause errors if it occurs.♻️ Suggested fix using useEffect cleanup
One approach is to track active listeners in a ref and clean them up on unmount:
+ const cleanupRef = useRef<(() => void) | null>(null); + + useEffect(() => { + return () => cleanupRef.current?.(); + }, []); const startPaneResize = useCallback( (handleIndex: 0 | 1) => (event: React.PointerEvent<HTMLButtonElement>) => { // ... existing code ... const up = () => { window.removeEventListener("pointermove", move); window.removeEventListener("pointerup", up); + cleanupRef.current = null; }; + cleanupRef.current = up; window.addEventListener("pointermove", move); window.addEventListener("pointerup", up, { once: true }); }, [paneCols], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/sandbox.tsx` around lines 493 - 532, The startPaneResize handler attaches window listeners (pointermove and pointerup) that aren't removed if the component unmounts mid-drag; add a cleanup to ensure removal by storing the active move and up callbacks in a ref (e.g., a ref named activeDragRef) when you call window.addEventListener inside startPaneResize, and add a useEffect with a cleanup that checks activeDragRef and removes those listeners (window.removeEventListener("pointermove", move) and window.removeEventListener("pointerup", up)) on unmount; ensure you still call event.currentTarget.setPointerCapture and keep the existing once option for pointerup but also clear listeners from the ref in the up handler so the cleanup knows there’s nothing to remove.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@website/src/components/sandbox.tsx`:
- Around line 493-532: The startPaneResize handler attaches window listeners
(pointermove and pointerup) that aren't removed if the component unmounts
mid-drag; add a cleanup to ensure removal by storing the active move and up
callbacks in a ref (e.g., a ref named activeDragRef) when you call
window.addEventListener inside startPaneResize, and add a useEffect with a
cleanup that checks activeDragRef and removes those listeners
(window.removeEventListener("pointermove", move) and
window.removeEventListener("pointerup", up)) on unmount; ensure you still call
event.currentTarget.setPointerCapture and keep the existing once option for
pointerup but also clear listeners from the ref in the up handler so the cleanup
knows there’s nothing to remove.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c9689226-a054-4db5-8f7d-31f49221432c
📒 Files selected for processing (4)
website/scripts/compute-llm-call-tokens.mjswebsite/src/components/sandbox.tsxwebsite/src/lib/tool-call-comparison.d.mtswebsite/src/lib/tool-call-comparison.mjs
✅ Files skipped from review due to trivial changes (1)
- website/src/lib/tool-call-comparison.d.mts
🚧 Files skipped from review as they are similar to previous changes (1)
- website/scripts/compute-llm-call-tokens.mjs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/src/components/sandbox.tsx (1)
728-746: Consider usingrole="separator"for the resizer elements.The
<hr>elements witharia-orientation,aria-valuemin/max/nowfollow the ARIA slider pattern, but splitter/resizer controls are typically marked withrole="separator"(when focusable) per the WAI-ARIA spec. This isn't blocking, but addingrole="separator"would make the intent clearer to assistive technologies.💡 Optional: Add explicit separator role
<hr className="sb-resizer" + role="separator" tabIndex={0} aria-orientation="horizontal"Also applies to: 760-778
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/sandbox.tsx` around lines 728 - 746, Add an explicit ARIA role to the resizer element to reflect it's a focusable separator: update the <hr> that uses className "sb-resizer" (the element wired to startPaneResize(0) and handlePaneResizeKeyDown(0), and using MIN_PANE_SIZE and paneCols to compute aria-valuemin/valuemax/valuenow) to include role="separator". Do the same change for the other symmetric resizer instance (the one around lines 760-778) so assistive tech recognizes the control as a separator while preserving the existing aria-* attributes and event handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@website/src/components/sandbox.tsx`:
- Around line 728-746: Add an explicit ARIA role to the resizer element to
reflect it's a focusable separator: update the <hr> that uses className
"sb-resizer" (the element wired to startPaneResize(0) and
handlePaneResizeKeyDown(0), and using MIN_PANE_SIZE and paneCols to compute
aria-valuemin/valuemax/valuenow) to include role="separator". Do the same change
for the other symmetric resizer instance (the one around lines 760-778) so
assistive tech recognizes the control as a separator while preserving the
existing aria-* attributes and event handlers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2de8252f-300d-4b6a-8dd6-47af3b33f89b
📒 Files selected for processing (2)
website/src/app/globals.csswebsite/src/components/sandbox.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
website/src/app/globals.css (1)
2625-2628: Keep an explicit focus ring on the splitter.The separator is now keyboard-resizable in
website/src/components/sandbox.tsx, but this rule removes the outline on:focus-visible. On a 7px handle, the background tint alone is easy to miss. Keep a real focus indicator here instead of zeroing it out.💡 Suggested tweak
.sb-resizer:hover, .sb-resizer:focus-visible { background: color-mix(in oklab, var(--accent) 22%, var(--paper-2)); - outline: 0; + } + .sb-resizer:focus-visible { + outline: 2px solid var(--accent); + outline-offset: -1px; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/app/globals.css` around lines 2625 - 2628, The focus-visible rule for the splitter (.sb-resizer:focus-visible) removes the outline which hides keyboard focus on the small 7px handle; instead of setting outline: 0, restore a visible focus indicator — update the .sb-resizer:focus-visible selector to remove the outline: 0 and apply an explicit, accessible focus style (for example an outline or focus ring using outline or box-shadow with a high-contrast color variable like --accent or a dedicated --focus variable) so keyboard users can clearly see the splitter is focused while preserving the existing hover styling in .sb-resizer:hover.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/src/components/sandbox.tsx`:
- Around line 543-573: The pointermove listener isn't removed if the gesture is
canceled or pointer capture is lost; capture the element and pointerId from the
initial event (e.g., const el = event.currentTarget as HTMLElement; const pid =
event.pointerId), then update the teardown to remove pointermove and pointerup
and also remove pointercancel and lostpointercapture listeners and call
el.releasePointerCapture(pid) during cleanup; add
window.addEventListener("pointercancel", cleanup) and
el.addEventListener("lostpointercapture", cleanup) (or similar) to ensure move
and up handlers created in the sandbox.tsx resize code (the move, up functions
and the resizePanePair call sites) are always detached.
---
Nitpick comments:
In `@website/src/app/globals.css`:
- Around line 2625-2628: The focus-visible rule for the splitter
(.sb-resizer:focus-visible) removes the outline which hides keyboard focus on
the small 7px handle; instead of setting outline: 0, restore a visible focus
indicator — update the .sb-resizer:focus-visible selector to remove the outline:
0 and apply an explicit, accessible focus style (for example an outline or focus
ring using outline or box-shadow with a high-contrast color variable like
--accent or a dedicated --focus variable) so keyboard users can clearly see the
splitter is focused while preserving the existing hover styling in
.sb-resizer:hover.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cddf44cb-6e7b-441e-ada4-d5b7bd9ed6b4
📒 Files selected for processing (2)
website/src/app/globals.csswebsite/src/components/sandbox.tsx
Summary