beta tag our playground#1968
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Internal previewPreview URL: https://mcp-inspector-pr-1968.up.railway.app |
WalkthroughThe changes update the sidebar navigation to always display the Evaluate section, removing the previous feature flag gate. A new feature flag now controls Playground availability. The Playground tab is marked as beta and includes conditional UI rendering for beta state, displaying a badge with tooltip when enabled or a "Coming soon" tooltip when locked. Related components gain optional props to manage beta visibility and locked states. Tests are updated to dynamically mock feature flags and validate the new Playground UI behavior across enabled and disabled scenarios. 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.
🧹 Nitpick comments (2)
mcpjam-inspector/client/src/components/mcp-sidebar.tsx (1)
754-756: Consider consolidating redundant props.Both
showPlaygroundBetaandplaygroundEnabledreceive the identical valueplaygroundEnabled === true. The component could deriveshowPlaygroundBetainternally fromplaygroundEnabled, reducing the API surface.♻️ Internal derivation approach
export function SidebarEvalsNavGroup({ title, Icon, disabled, disabledTooltip, activeTab, showRuns = true, - showPlaygroundBeta = false, playgroundEnabled = false, }: { // ... showRuns?: boolean; - showPlaygroundBeta?: boolean; playgroundEnabled?: boolean; }) { + const showPlaygroundBeta = playgroundEnabled; // ...And at the call site:
<SidebarEvalsNavGroup title={evalsEntry.title} Icon={evalsEntry.icon} disabled={evalsEntry.disabled} disabledTooltip={evalsEntry.disabledTooltip} activeTab={activeTab} showRuns={evaluateRunsEnabled === true} - showPlaygroundBeta={playgroundEnabled === true} playgroundEnabled={playgroundEnabled === true} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/mcp-sidebar.tsx` around lines 754 - 756, Both props showPlaygroundBeta and playgroundEnabled are being passed the same boolean expression; remove the redundant showPlaygroundBeta prop at the call site and derive it inside the receiving component from playgroundEnabled instead. Specifically, stop passing showPlaygroundBeta={playgroundEnabled === true} where playgroundEnabled is passed, update the receiving component's props/interface (remove showPlaygroundBeta or mark it optional), and compute const showPlaygroundBeta = Boolean(playgroundEnabled) (or playgroundEnabled === true) inside that component to use for conditional rendering. Ensure any TypeScript Prop types / defaultProps are updated to reflect the single source of truth (playgroundEnabled).mcpjam-inspector/client/src/components/sidebar/__tests__/sidebar-invite-cta.test.tsx (1)
273-276: PrefergetByRolefor element selection.Using
.closest("button") as HTMLButtonElementis fragile and relies on a type assertion. Testing Library's role-based queries are more resilient and semantically clearer.♻️ Suggested refinement
- const playground = screen - .getByText("Playground") - .closest("button") as HTMLButtonElement; - expect(playground).toBeInTheDocument(); + const playground = screen.getByRole("button", { name: "Playground" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/sidebar/__tests__/sidebar-invite-cta.test.tsx` around lines 273 - 276, The test currently locates the "Playground" button via screen.getByText("Playground").closest("button") and casts to HTMLButtonElement, which is fragile; replace that lookup with a role-based query such as screen.getByRole("button", { name: /Playground/i }) (or screen.getByRole("button", { name: "Playground" })) to directly find the button element and remove the .closest(...) and the type assertion—update the variable (playground) and the expect(playground).toBeInTheDocument() assertion accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@mcpjam-inspector/client/src/components/mcp-sidebar.tsx`:
- Around line 754-756: Both props showPlaygroundBeta and playgroundEnabled are
being passed the same boolean expression; remove the redundant
showPlaygroundBeta prop at the call site and derive it inside the receiving
component from playgroundEnabled instead. Specifically, stop passing
showPlaygroundBeta={playgroundEnabled === true} where playgroundEnabled is
passed, update the receiving component's props/interface (remove
showPlaygroundBeta or mark it optional), and compute const showPlaygroundBeta =
Boolean(playgroundEnabled) (or playgroundEnabled === true) inside that component
to use for conditional rendering. Ensure any TypeScript Prop types /
defaultProps are updated to reflect the single source of truth
(playgroundEnabled).
In
`@mcpjam-inspector/client/src/components/sidebar/__tests__/sidebar-invite-cta.test.tsx`:
- Around line 273-276: The test currently locates the "Playground" button via
screen.getByText("Playground").closest("button") and casts to HTMLButtonElement,
which is fragile; replace that lookup with a role-based query such as
screen.getByRole("button", { name: /Playground/i }) (or
screen.getByRole("button", { name: "Playground" })) to directly find the button
element and remove the .closest(...) and the type assertion—update the variable
(playground) and the expect(playground).toBeInTheDocument() assertion
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 94589d92-aa21-4ddb-96e6-334d8affce50
📒 Files selected for processing (2)
mcpjam-inspector/client/src/components/mcp-sidebar.tsxmcpjam-inspector/client/src/components/sidebar/__tests__/sidebar-invite-cta.test.tsx
No description provided.