Merged
Conversation
Contributor
✅ 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. |
dosubot
bot
added
the
size:XXL
Contributor
WalkthroughThis pull request refactors theme handling across the chat and UI components to support dynamic, sandbox-scoped theming. A new 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
mcpjam-inspector/client/src/components/sidebar/__tests__/nav-main.test.tsx (1)
5-16: The new mock deserves one learn-more assertion.
useSidebar: { open: true }keeps the old cases green, but it still leaves the newlearnMorebranch unexercised. A focused case for “visited + open shows the hover card, collapsed does not” would protect the behavior added inNavMain.🤖 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__/nav-main.test.tsx` around lines 5 - 16, Add a focused test that exercises the new learnMore branch by mocking useSidebar to toggle open/closed and asserting the learn-more hover card behavior in NavMain: when the item is marked visited and useSidebar returns { open: true } the hover card (learnMore) is present, and when useSidebar returns { open: false } it is not; update the test file nav-main.test.tsx to render NavMain (or the relevant SidebarMenuItem) with a visited state, stub the SidebarMenuButton/SidebarGroup mocks as shown, and include assertions for the learnMore hover card visibility for both open and collapsed states.mcpjam-inspector/client/src/components/learn-more/LearnMoreModal.tsx (1)
17-78: Extract the learn-more body before it forks.This component already repeats the same header, media, and docs-link rendering living in
LearnMoreExpandedPanel.tsx. Pulling that body into a shared child—or dropping one variant—will keep copy, fallbacks, and links from drifting apart.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/learn-more/LearnMoreModal.tsx` around lines 17 - 78, The LearnMoreModal duplicates the header/media/docs-link rendering that also exists in LearnMoreExpandedPanel; extract the repeated JSX into a new shared component (e.g., LearnMoreBody) that accepts the entry object (from learnMoreContent) and any callbacks needed, implement the same conditional video vs iframe and "Video coming soon" fallback, and render the docs Button with entry.docsUrl inside it; then import and use LearnMoreBody in both LearnMoreModal (replace the header/media/docs JSX block inside DialogContent) and LearnMoreExpandedPanel, keeping each parent responsible only for surrounding Dialog/DialogContent or panel layout so the shared logic for header, media, fallbacks, and docs link lives in one place.mcpjam-inspector/client/src/components/shared/__tests__/DisplayContextHeader.test.tsx (1)
68-161: Use the shared client test presets instead of bespoke store mocks.This suite hand-rolls the preferences, playground, and client-config stores, which makes the harness easier to drift from the repo standard. Please seed the state from the shared presets and override only what this scenario needs. As per coding guidelines,
mcpjam-inspector/client/**/__tests__/*.test.{ts,tsx}: Use mock presets fromclient/src/test/mocks/includingmcpApiPresetsandstorePresetsin client tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/shared/__tests__/DisplayContextHeader.test.tsx` around lines 68 - 161, Replace the handwritten store mocks with the shared test presets: import the shared presets (e.g., mcpApiPresets and storePresets from client/src/test/mocks/) and use those to seed the stores instead of the bespoke mock objects created in the vi.hoisted block; specifically stop manually mocking usePreferencesStore, useUIPlaygroundStore, useClientConfigStore and their state (mockPreferencesState, mockUIPlaygroundStore, mockPatchHostContext) and instead call the shared store preset and override only the few fields this test needs (themeMode, deviceType/customViewport, draftConfig.hostContext, patchHostContext), while keeping the existing vi.mock for updateThemeMode and any other utility mocks intact.mcpjam-inspector/client/src/components/ui-playground/__tests__/PlaygroundMain.test.tsx (1)
299-375: Swap the hand-rolled client mocks for the shared presets.This file builds its own preferences and UI-playground fixtures instead of the repository presets, so the harness can quietly diverge from the standard client setup. Please base these mocks on the shared presets and override only the fields this suite exercises. As per coding guidelines,
mcpjam-inspector/client/**/__tests__/*.test.{ts,tsx}: Use mock presets fromclient/src/test/mocks/includingmcpApiPresetsandstorePresetsin client tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/ui-playground/__tests__/PlaygroundMain.test.tsx` around lines 299 - 375, Replace the hand-rolled mock objects with the shared test presets: import the repository presets (e.g., storePresets and mcpApiPresets) from client/src/test/mocks and use them inside the vi.mock implementations for usePreferencesStore and useUIPlaygroundStore instead of mockPreferencesState and mockUIPlaygroundStore; keep the same selector pattern (selector ? selector(presets.xxx) : presets.xxx) and only override the specific properties the tests exercise (e.g., themeMode, themePreset, deviceType, customViewport, capabilities) while preserving PRESET_DEVICE_CONFIGS and the DisplayContextHeader mock/export names so tests still see PRESET_DEVICE_CONFIGS and DisplayContextHeader with the theme toggle behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcpjam-inspector/client/src/components/chat-v2/__tests__/ChatInput.test.tsx`:
- Around line 10-13: The test currently inlines usePreferencesStore which
bypasses the shared test harness; replace the inline mock with the shared store
preset from the client test mocks so the suite uses the same store shape. Remove
the current vi.mock implementation for usePreferencesStore and instead apply the
shared store preset (storePresets) together with any existing mcpApiPresets used
in tests so the preference state (and other stores) are provided consistently;
locate the mock setup in ChatInput.test.tsx where usePreferencesStore is mocked
and swap it to import/apply storePresets (and mcpApiPresets if needed) from the
test mocks.
- Around line 172-184: The test currently asserts classes that are hardcoded in
ChatInput so it doesn't verify host-theme wiring; update the test to render
ChatInput with SandboxHostStyleProvider and SandboxHostThemeProvider for both
"dark" and "light" (or at least dark vs not-dark) and assert on a DOM change
that actually reflects the resolved host theme — for example check for a
host-theme-specific attribute or class applied to the composer wrapper (look for
a data attribute or class added by SandboxHostThemeProvider on the ChatInput
root/wrapper element) or assert that the wrapper/textarea toggles a
theme-dependent class (use the ChatInput root/wrapper element or the textarea
obtained via getByPlaceholderText and verify presence/absence of that actual
theme-controlled attribute/class rather than bg-transparent).
In
`@mcpjam-inspector/client/src/components/chat-v2/chat-input/model/provider-logo.tsx`:
- Around line 33-40: The fallback icon div rendered in the ProviderLogo
component is invisible because it only receives text-color classes from
getProviderColorForTheme(provider, resolvedThemeMode) and has no background;
update the className for that div (the JSX return that uses cn("h-3 w-3
rounded-sm", getProviderColorForTheme(...))) to include a background utility
such as "bg-current" (matching the custom provider's use of "bg-primary/10") so
the element is visible; modify the class list where ProviderLogo renders the
fallback div to add the background class.
In
`@mcpjam-inspector/client/src/components/learn-more/LearnMoreExpandedPanel.tsx`:
- Around line 13-15: The code currently uses the constant PANEL_WIDTH (720) when
computing finalX, scaleX and marginLeft which breaks on narrow viewports; change
those calculations in LearnMoreExpandedPanel to derive an actual panelWidth at
runtime (e.g., min(PANEL_WIDTH, computed sheet/clientWidth or window.innerWidth
minus desired padding) or read sheetRef.getBoundingClientRect().width) and use
that panelWidth variable wherever finalX, scaleX, and marginLeft are computed
(replace direct references to PANEL_WIDTH in the calculations around finalX,
scaleX, and marginLeft in the component). Ensure these computations run on
mount/resize so the opening animation targets the rendered width and update any
related animation/easing code paths in the same component blocks (the areas
previously using PANEL_WIDTH at the indicated ranges).
- Around line 67-112: The overlay/panel lacks modal semantics and focus
management: update LearnMoreExpandedPanel.tsx to make the panel a proper dialog
(add role="dialog", aria-modal="true" and an aria-labelledby/id for the title),
move focus into the panel on open (use panelRef or the close button as the
initial focus target), trap focus while open (implement a focus trap using
key/Tab handling and focusable element queries tied to panelRef), restore the
previously focused element on close (use a saved activeElement and call focus()
in the onClose path), and ensure the close button (XIcon wrapper) has a visible
focus style (add a focus-visible class or inline style). Ensure getInitialStyle,
panelRef and onClose are used to locate where to wire these behaviors and escape
closes the dialog.
In `@mcpjam-inspector/client/src/components/shared/DisplayContextHeader.tsx`:
- Around line 222-231: The non-override branch in handleThemeChange currently
only calls updateThemeMode and setThemeMode but does not update the hostContext
bridge that extractHostTheme(draftHostContext) is read from; modify the fallback
path so after computing newTheme you also patch/update the
hostContext/draftHostContext to reflect the new theme (so
extractHostTheme(draftHostContext) will return newTheme) — e.g., call the
existing host-context update/patch function or set the draftHostContext theme
field alongside updateThemeMode and setThemeMode; keep the override branch
(onThemeToggleOverride) unchanged.
In `@mcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsx`:
- Around line 807-840: When xrayMode is true the branch renders XRaySnapshotView
plus a ChatInput footer and hides the main thread where ErrorBox lives, so
submit/auth errors disappear; fix by extracting the shared composer+error UI
into a single reusable component (e.g., ComposerWithErrors) that composes
ErrorBox and ChatInput, then render that component both in the XRay footer
(inside the StickToBottom block) and in the threadContent area (or move
ChatInput to be mounted once outside the xrayMode conditional and reuse it),
ensuring ErrorBox (the existing error surface) is included in the extracted
component so errors remain visible while X-Ray is open and you don’t mount a
second ChatInput instance.
- Around line 789-792: The shell-level dark scoping uses the className
expression with effectiveThreadTheme and .app-theme-scope but does not prevent
Tailwind `dark:` utilities from matching a higher-level html.dark; update the
sandbox host to either apply a CSS reset that neutralizes inherited `dark:`
selectors when effectiveThreadTheme !== "dark" or convert hardcoded `dark:`
color utilities to CSS variables exposed by .app-theme-scope so they follow the
theme flag. Locate the JSX that sets className (the className call wrapping
"sandbox-host-shell app-theme-scope ..." and the effectiveThreadTheme check) and
implement one of two fixes: wrap the light shell with a reset scope that
disables `dark:` matching, or replace descendant hardcoded `dark:...` usages
(e.g., in user-message-bubble.tsx and chat-input.tsx) with theme-aware CSS
variables defined on .app-theme-scope so dark colors are applied only when
effectiveThreadTheme === "dark".
---
Nitpick comments:
In `@mcpjam-inspector/client/src/components/learn-more/LearnMoreModal.tsx`:
- Around line 17-78: The LearnMoreModal duplicates the header/media/docs-link
rendering that also exists in LearnMoreExpandedPanel; extract the repeated JSX
into a new shared component (e.g., LearnMoreBody) that accepts the entry object
(from learnMoreContent) and any callbacks needed, implement the same conditional
video vs iframe and "Video coming soon" fallback, and render the docs Button
with entry.docsUrl inside it; then import and use LearnMoreBody in both
LearnMoreModal (replace the header/media/docs JSX block inside DialogContent)
and LearnMoreExpandedPanel, keeping each parent responsible only for surrounding
Dialog/DialogContent or panel layout so the shared logic for header, media,
fallbacks, and docs link lives in one place.
In
`@mcpjam-inspector/client/src/components/shared/__tests__/DisplayContextHeader.test.tsx`:
- Around line 68-161: Replace the handwritten store mocks with the shared test
presets: import the shared presets (e.g., mcpApiPresets and storePresets from
client/src/test/mocks/) and use those to seed the stores instead of the bespoke
mock objects created in the vi.hoisted block; specifically stop manually mocking
usePreferencesStore, useUIPlaygroundStore, useClientConfigStore and their state
(mockPreferencesState, mockUIPlaygroundStore, mockPatchHostContext) and instead
call the shared store preset and override only the few fields this test needs
(themeMode, deviceType/customViewport, draftConfig.hostContext,
patchHostContext), while keeping the existing vi.mock for updateThemeMode and
any other utility mocks intact.
In `@mcpjam-inspector/client/src/components/sidebar/__tests__/nav-main.test.tsx`:
- Around line 5-16: Add a focused test that exercises the new learnMore branch
by mocking useSidebar to toggle open/closed and asserting the learn-more hover
card behavior in NavMain: when the item is marked visited and useSidebar returns
{ open: true } the hover card (learnMore) is present, and when useSidebar
returns { open: false } it is not; update the test file nav-main.test.tsx to
render NavMain (or the relevant SidebarMenuItem) with a visited state, stub the
SidebarMenuButton/SidebarGroup mocks as shown, and include assertions for the
learnMore hover card visibility for both open and collapsed states.
In
`@mcpjam-inspector/client/src/components/ui-playground/__tests__/PlaygroundMain.test.tsx`:
- Around line 299-375: Replace the hand-rolled mock objects with the shared test
presets: import the repository presets (e.g., storePresets and mcpApiPresets)
from client/src/test/mocks and use them inside the vi.mock implementations for
usePreferencesStore and useUIPlaygroundStore instead of mockPreferencesState and
mockUIPlaygroundStore; keep the same selector pattern (selector ?
selector(presets.xxx) : presets.xxx) and only override the specific properties
the tests exercise (e.g., themeMode, themePreset, deviceType, customViewport,
capabilities) while preserving PRESET_DEVICE_CONFIGS and the
DisplayContextHeader mock/export names so tests still see PRESET_DEVICE_CONFIGS
and DisplayContextHeader with the theme toggle behavior.
🪄 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: 4c443a70-c0e5-4eeb-a2d7-a3670ec69919
⛔ Files ignored due to path filters (1)
mcpjam-inspector/client/public/tool-vid-march.mp4is excluded by!**/*.mp4
📒 Files selected for processing (31)
mcpjam-inspector/client/src/components/chat-v2/__tests__/ChatInput.test.tsxmcpjam-inspector/client/src/components/chat-v2/__tests__/MessageView.test.tsxmcpjam-inspector/client/src/components/chat-v2/__tests__/ThinkingIndicator.test.tsxmcpjam-inspector/client/src/components/chat-v2/chat-input.tsxmcpjam-inspector/client/src/components/chat-v2/chat-input/model/provider-logo.tsxmcpjam-inspector/client/src/components/chat-v2/shared/assistant-avatar.tsmcpjam-inspector/client/src/components/chat-v2/shared/chat-helpers.tsmcpjam-inspector/client/src/components/chat-v2/shared/thinking-indicator.tsxmcpjam-inspector/client/src/components/chat-v2/thread/csp-debug-panel.tsxmcpjam-inspector/client/src/components/chat-v2/thread/message-view.tsxmcpjam-inspector/client/src/components/chat-v2/thread/parts/tool-part.tsxmcpjam-inspector/client/src/components/chat-v2/thread/user-message-bubble.tsxmcpjam-inspector/client/src/components/learn-more/LearnMoreExpandedPanel.tsxmcpjam-inspector/client/src/components/learn-more/LearnMoreHoverCard.tsxmcpjam-inspector/client/src/components/learn-more/LearnMoreModal.tsxmcpjam-inspector/client/src/components/mcp-sidebar.tsxmcpjam-inspector/client/src/components/shared/DisplayContextHeader.tsxmcpjam-inspector/client/src/components/shared/__tests__/DisplayContextHeader.test.tsxmcpjam-inspector/client/src/components/sidebar/__tests__/nav-main.test.tsxmcpjam-inspector/client/src/components/sidebar/nav-main.tsxmcpjam-inspector/client/src/components/ui-playground/PlaygroundLeft.tsxmcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsxmcpjam-inspector/client/src/components/ui-playground/TabHeader.tsxmcpjam-inspector/client/src/components/ui-playground/__tests__/PlaygroundMain.test.tsxmcpjam-inspector/client/src/contexts/sandbox-host-style-context.tsmcpjam-inspector/client/src/hooks/use-learn-more.tsmcpjam-inspector/client/src/index.cssmcpjam-inspector/client/src/lib/learn-more-content.tsmcpjam-inspector/client/src/styles/presets/brutalist.cssmcpjam-inspector/client/src/styles/presets/soft-pop.cssmcpjam-inspector/client/src/styles/presets/tangerine.css
Comment on lines
+10
to
+13
| vi.mock("@/stores/preferences/preferences-provider", () => ({ | ||
| usePreferencesStore: (selector: (state: { themeMode: "light" }) => unknown) => | ||
| selector({ themeMode: "light" }), | ||
| })); |
Contributor
There was a problem hiding this comment.
Use the shared client store preset here.
Inlining usePreferencesStore in this file bypasses the standard client-test harness and only models themeMode. Please switch this to the shared store preset so the test environment stays aligned with the rest of the suite.
As per coding guidelines, mcpjam-inspector/client/**/__tests__/*.test.{ts,tsx}: Use mock presets from client/src/test/mocks/ including mcpApiPresets and storePresets in client tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/components/chat-v2/__tests__/ChatInput.test.tsx`
around lines 10 - 13, The test currently inlines usePreferencesStore which
bypasses the shared test harness; replace the inline mock with the shared store
preset from the client test mocks so the suite uses the same store shape. Remove
the current vi.mock implementation for usePreferencesStore and instead apply the
shared store preset (storePresets) together with any existing mcpApiPresets used
in tests so the preference state (and other stores) are provided consistently;
locate the mock setup in ChatInput.test.tsx where usePreferencesStore is mocked
and swap it to import/apply storePresets (and mcpApiPresets if needed) from the
test mocks.
mcpjam-inspector/client/src/components/chat-v2/__tests__/ChatInput.test.tsx
Show resolved
Hide resolved
mcpjam-inspector/client/src/components/chat-v2/chat-input/model/provider-logo.tsx
Show resolved
Hide resolved
Comment on lines
+13
to
+15
| const PANEL_WIDTH = 720; | ||
| const EASING: [number, number, number, number] = [0.16, 1, 0.3, 1]; // ease-out-expo | ||
|
|
Contributor
There was a problem hiding this comment.
Center the panel with its rendered width.
On narrow viewports, maxWidth shrinks the sheet while finalX, scaleX, and marginLeft still assume 720px. The result is a panel that can sit partly off-screen, with an opening animation that targets the wrong destination.
Suggested fix
-const PANEL_WIDTH = 720;
+const PANEL_MAX_WIDTH = 720;
@@
- const finalX = (viewW - PANEL_WIDTH) / 2;
+ const panelWidth = Math.max(1, Math.min(PANEL_MAX_WIDTH, viewW - 32));
+ const finalX = (viewW - panelWidth) / 2;
@@
- const scaleX = sourceRect.width / PANEL_WIDTH;
+ const scaleX = sourceRect.width / panelWidth;
@@
style={{
top: "10vh",
left: "50%",
- marginLeft: -(PANEL_WIDTH / 2),
- width: PANEL_WIDTH,
- maxWidth: "calc(100vw - 2rem)",
+ width: `min(${PANEL_MAX_WIDTH}px, calc(100vw - 2rem))`,
+ transform: "translateX(-50%)",
maxHeight: "80vh",
}}Also applies to: 41-46, 51-57, 82-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/components/learn-more/LearnMoreExpandedPanel.tsx`
around lines 13 - 15, The code currently uses the constant PANEL_WIDTH (720)
when computing finalX, scaleX and marginLeft which breaks on narrow viewports;
change those calculations in LearnMoreExpandedPanel to derive an actual
panelWidth at runtime (e.g., min(PANEL_WIDTH, computed sheet/clientWidth or
window.innerWidth minus desired padding) or read
sheetRef.getBoundingClientRect().width) and use that panelWidth variable
wherever finalX, scaleX, and marginLeft are computed (replace direct references
to PANEL_WIDTH in the calculations around finalX, scaleX, and marginLeft in the
component). Ensure these computations run on mount/resize so the opening
animation targets the rendered width and update any related animation/easing
code paths in the same component blocks (the areas previously using PANEL_WIDTH
at the indicated ranges).
Comment on lines
+67
to
+112
| <motion.div | ||
| key="learn-more-overlay" | ||
| className="fixed inset-0 z-50 bg-black/50" | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| exit={{ opacity: 0 }} | ||
| transition={{ duration: 0.25, ease: EASING }} | ||
| onClick={onClose} | ||
| /> | ||
|
|
||
| {/* Panel */} | ||
| <motion.div | ||
| ref={panelRef} | ||
| key="learn-more-panel" | ||
| className="fixed z-50 bg-background rounded-lg border shadow-lg p-6 overflow-y-auto" | ||
| style={{ | ||
| top: "10vh", | ||
| left: "50%", | ||
| marginLeft: -(PANEL_WIDTH / 2), | ||
| width: PANEL_WIDTH, | ||
| maxWidth: "calc(100vw - 2rem)", | ||
| maxHeight: "80vh", | ||
| }} | ||
| initial={getInitialStyle()} | ||
| animate={{ | ||
| opacity: 1, | ||
| x: 0, | ||
| y: 0, | ||
| scale: 1, | ||
| transformOrigin: "top left", | ||
| }} | ||
| exit={{ | ||
| opacity: 0, | ||
| scale: 0.97, | ||
| transition: { duration: 0.15, ease: EASING } as any, | ||
| }} | ||
| transition={{ duration: 0.35, ease: EASING }} | ||
| > | ||
| {/* Close button */} | ||
| <button | ||
| onClick={onClose} | ||
| className="absolute top-4 right-4 rounded-xs opacity-70 transition-opacity hover:opacity-100 focus:outline-none" | ||
| > | ||
| <XIcon className="h-4 w-4" /> | ||
| <span className="sr-only">Close</span> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Give this overlay real modal semantics.
This panel auto-opens on first navigation, but focus never moves into it and the background remains tabbable. Keyboard and screen-reader users can keep interacting with obscured content instead of the new surface.
At minimum, add dialog semantics, move focus into the panel on open, trap it while open, and restore it on close. If this stays custom, it also needs a visible focus style on the close button.
Also applies to: 114-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/components/learn-more/LearnMoreExpandedPanel.tsx`
around lines 67 - 112, The overlay/panel lacks modal semantics and focus
management: update LearnMoreExpandedPanel.tsx to make the panel a proper dialog
(add role="dialog", aria-modal="true" and an aria-labelledby/id for the title),
move focus into the panel on open (use panelRef or the close button as the
initial focus target), trap focus while open (implement a focus trap using
key/Tab handling and focusable element queries tied to panelRef), restore the
previously focused element on close (use a saved activeElement and call focus()
in the onClose path), and ensure the close button (XIcon wrapper) has a visible
focus style (add a focus-visible class or inline style). Ensure getInitialStyle,
panelRef and onClose are used to locate where to wire these behaviors and escape
closes the dialog.
Comment on lines
222
to
+231
| const handleThemeChange = useCallback(() => { | ||
| const newTheme = theme === "dark" ? "light" : "dark"; | ||
| patchHostContext({ theme: newTheme }); | ||
| }, [theme, patchHostContext]); | ||
| if (usesThemeOverride && onThemeToggleOverride) { | ||
| onThemeToggleOverride(); | ||
| return; | ||
| } | ||
|
|
||
| const newTheme = themeMode === "dark" ? "light" : "dark"; | ||
| updateThemeMode(newTheme); | ||
| setThemeMode(newTheme); | ||
| }, [onThemeToggleOverride, setThemeMode, themeMode, usesThemeOverride]); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for preview theme readers/writers that still depend on hostContext..."
rg -n -C2 -t ts -t tsx '\bextractHostTheme\s*\(|patchHostContext\(\{\s*theme\b' mcpjam-inspector/client/src || trueRepository: MCPJam/inspector
Length of output: 184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Searching for hostContext theme usage patterns ==="
rg -n -C2 'extractHostTheme|patchHostContext.*theme|hostContext.*theme' mcpjam-inspector/client/src || echo "No direct matches found"
echo ""
echo "=== Searching for hostContext references broadly ==="
rg -n 'hostContext' mcpjam-inspector/client/src | head -30 || echo "No hostContext usage found"
echo ""
echo "=== Checking DisplayContextHeader for updateThemeMode/setThemeMode calls ==="
rg -n -A5 'updateThemeMode|setThemeMode' mcpjam-inspector/client/src/components/shared/DisplayContextHeader.tsx || echo "No theme update calls"
echo ""
echo "=== Searching for usePreferencesStore theme usage ==="
rg -n 'usePreferencesStore.*theme|updateThemeMode' mcpjam-inspector/client/src | head -20 || echo "No preferences theme usage"Repository: MCPJam/inspector
Length of output: 10145
Update the non-override path to bridge theme changes into hostContext.
The fallback branch updates only the global preference store. Since chatgpt-app-renderer and mcp-apps-renderer consume theme from extractHostTheme(draftHostContext) with a fallback to themeMode, the non-override path must also patch hostContext to ensure those shared preview renderers react to the toggle. The override path (onThemeToggleOverride()) bypasses this concern, but the fallback needs to maintain the hostContext bridge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/components/shared/DisplayContextHeader.tsx`
around lines 222 - 231, The non-override branch in handleThemeChange currently
only calls updateThemeMode and setThemeMode but does not update the hostContext
bridge that extractHostTheme(draftHostContext) is read from; modify the fallback
path so after computing newTheme you also patch/update the
hostContext/draftHostContext to reflect the new theme (so
extractHostTheme(draftHostContext) will return newTheme) — e.g., call the
existing host-context update/patch function or set the draftHostContext theme
field alongside updateThemeMode and setThemeMode; keep the override branch
(onThemeToggleOverride) unchanged.
Comment on lines
+807
to
+840
| {xrayMode && ( | ||
| <StickToBottom | ||
| className="relative flex flex-1 flex-col min-h-0" | ||
| resize="smooth" | ||
| initial="smooth" | ||
| > | ||
| <div className="relative flex-1 min-h-0"> | ||
| <StickToBottom.Content className="flex flex-col min-h-0"> | ||
| <XRaySnapshotView | ||
| systemPrompt={systemPrompt} | ||
| messages={messages} | ||
| selectedServers={selectedServers} | ||
| onClose={() => setXrayMode(false)} | ||
| /> | ||
| </StickToBottom.Content> | ||
| <ScrollToBottomButton /> | ||
| </div> | ||
| <div className="flex-shrink-0 border-t border-border"> | ||
| <div className="max-w-xl mx-auto w-full p-3"> | ||
| <ChatInput | ||
| {...sharedChatInputProps} | ||
| hasMessages={!isThreadEmpty} | ||
| /> | ||
| </div> | ||
| </div> | ||
| </StickToBottom> | ||
| )} | ||
| {/* Thread: kept mounted (but hidden) during X-Ray to preserve | ||
| MCPAppsRenderer iframes and bridge connections */} | ||
| <div | ||
| className="flex flex-col flex-1 min-h-0" | ||
| style={xrayMode ? { display: "none" } : undefined} | ||
| > | ||
| {threadContent} |
Contributor
There was a problem hiding this comment.
X-Ray mode hides the only send-error surface.
When xrayMode is on, the subtree that contains the ErrorBox on Lines 693-705 is set to display: none, and the replacement branch renders only XRaySnapshotView plus ChatInput. Submit and auth errors disappear while X-Ray is open. Please mirror the error block into this footer, or extract a shared composer/error section; that would also avoid mounting a second ChatInput.
Patch sketch
<div className="max-w-xl mx-auto w-full p-3">
+ {errorMessage && (
+ <div className="pb-3">
+ <ErrorBox
+ message={errorMessage.message}
+ errorDetails={errorMessage.details}
+ code={errorMessage.code}
+ statusCode={errorMessage.statusCode}
+ isRetryable={errorMessage.isRetryable}
+ isMCPJamPlatformError={errorMessage.isMCPJamPlatformError}
+ onResetChat={resetChat}
+ />
+ </div>
+ )}
<ChatInput
{...sharedChatInputProps}
hasMessages={!isThreadEmpty}📝 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.
Suggested change
| {xrayMode && ( | |
| <StickToBottom | |
| className="relative flex flex-1 flex-col min-h-0" | |
| resize="smooth" | |
| initial="smooth" | |
| > | |
| <div className="relative flex-1 min-h-0"> | |
| <StickToBottom.Content className="flex flex-col min-h-0"> | |
| <XRaySnapshotView | |
| systemPrompt={systemPrompt} | |
| messages={messages} | |
| selectedServers={selectedServers} | |
| onClose={() => setXrayMode(false)} | |
| /> | |
| </StickToBottom.Content> | |
| <ScrollToBottomButton /> | |
| </div> | |
| <div className="flex-shrink-0 border-t border-border"> | |
| <div className="max-w-xl mx-auto w-full p-3"> | |
| <ChatInput | |
| {...sharedChatInputProps} | |
| hasMessages={!isThreadEmpty} | |
| /> | |
| </div> | |
| </div> | |
| </StickToBottom> | |
| )} | |
| {/* Thread: kept mounted (but hidden) during X-Ray to preserve | |
| MCPAppsRenderer iframes and bridge connections */} | |
| <div | |
| className="flex flex-col flex-1 min-h-0" | |
| style={xrayMode ? { display: "none" } : undefined} | |
| > | |
| {threadContent} | |
| {xrayMode && ( | |
| <StickToBottom | |
| className="relative flex flex-1 flex-col min-h-0" | |
| resize="smooth" | |
| initial="smooth" | |
| > | |
| <div className="relative flex-1 min-h-0"> | |
| <StickToBottom.Content className="flex flex-col min-h-0"> | |
| <XRaySnapshotView | |
| systemPrompt={systemPrompt} | |
| messages={messages} | |
| selectedServers={selectedServers} | |
| onClose={() => setXrayMode(false)} | |
| /> | |
| </StickToBottom.Content> | |
| <ScrollToBottomButton /> | |
| </div> | |
| <div className="flex-shrink-0 border-t border-border"> | |
| <div className="max-w-xl mx-auto w-full p-3"> | |
| {errorMessage && ( | |
| <div className="pb-3"> | |
| <ErrorBox | |
| message={errorMessage.message} | |
| errorDetails={errorMessage.details} | |
| code={errorMessage.code} | |
| statusCode={errorMessage.statusCode} | |
| isRetryable={errorMessage.isRetryable} | |
| isMCPJamPlatformError={errorMessage.isMCPJamPlatformError} | |
| onResetChat={resetChat} | |
| /> | |
| </div> | |
| )} | |
| <ChatInput | |
| {...sharedChatInputProps} | |
| hasMessages={!isThreadEmpty} | |
| /> | |
| </div> | |
| </div> | |
| </StickToBottom> | |
| )} | |
| {/* Thread: kept mounted (but hidden) during X-Ray to preserve | |
| MCPAppsRenderer iframes and bridge connections */} | |
| <div | |
| className="flex flex-col flex-1 min-h-0" | |
| style={xrayMode ? { display: "none" } : undefined} | |
| > | |
| {threadContent} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsx`
around lines 807 - 840, When xrayMode is true the branch renders
XRaySnapshotView plus a ChatInput footer and hides the main thread where
ErrorBox lives, so submit/auth errors disappear; fix by extracting the shared
composer+error UI into a single reusable component (e.g., ComposerWithErrors)
that composes ErrorBox and ChatInput, then render that component both in the
XRay footer (inside the StickToBottom block) and in the threadContent area (or
move ChatInput to be mounted once outside the xrayMode conditional and reuse
it), ensuring ErrorBox (the existing error surface) is included in the extracted
component so errors remain visible while X-Ray is open and you don’t mount a
second ChatInput instance.
ignaciojimenezr
force-pushed
the
thread-colors
branch
from
02340a3 to
bfd50d5
Compare
dosubot
bot
added
size:XL
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
mcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsx (1)
806-840:⚠️ Potential issue | 🟠 MajorX-Ray mode still drops the only error surface.
When
xrayModeis on, the regular composer block withErrorBoxis hidden, and this replacement footer renders onlyChatInput. Submit and auth errors disappear while X-Ray is open, and a second composer stays mounted underneath. Reuse the same composer + error block here, or mirrorErrorBoxinto the X-Ray footer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsx` around lines 806 - 840, When xrayMode is true the footer renders only ChatInput so submit/auth errors (rendered by ErrorBox) are hidden; update the X-Ray footer inside the StickToBottom block (where XRaySnapshotView and ChatInput are used) to reuse the same composer + ErrorBox as the regular thread footer instead of rendering only ChatInput. Locate the XRaySnapshotView/StickToBottom block and replace the inner footer panel that currently renders ChatInput with the same composer container used for threadContent (including ErrorBox and the composer component referenced by sharedChatInputProps/ChatInput) or mirror ErrorBox into that footer so submit and auth errors remain visible while xrayMode is active.mcpjam-inspector/client/src/components/chat-v2/__tests__/ChatInput.test.tsx (2)
10-13:⚠️ Potential issue | 🟡 MinorUse shared client store presets instead of inline
usePreferencesStoremock.Inlining the preferences store bypasses the standard client test harness and drifts from repo test conventions.
As per coding guidelines,
mcpjam-inspector/client/**/__tests__/*.test.{ts,tsx}: Use mock presets fromclient/src/test/mocks/includingmcpApiPresetsandstorePresetsin client tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/chat-v2/__tests__/ChatInput.test.tsx` around lines 10 - 13, Replace the inline mock of usePreferencesStore in the test with the shared test store presets: remove the vi.mock block that defines usePreferencesStore and instead import and apply the existing storePresets (and mcpApiPresets if used) from the test mocks in the test setup; ensure the test uses storePresets to provide the themeMode ("light") value so references to usePreferencesStore in the ChatInput tests resolve via the standard client test harness rather than an inline mock.
172-185:⚠️ Potential issue | 🟡 MinorThis assertion still doesn’t prove host-theme wiring.
Checking
bg-transparent/dark:bg-transparentis non-discriminating here; the test can pass even if scoped theme resolution regresses. Assert a DOM signal that truly toggles with resolved host theme.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/chat-v2/__tests__/ChatInput.test.tsx` around lines 172 - 185, The test currently only checks for CSS utility class names which can pass even if host-theme wiring breaks; update the test for ChatInput inside SandboxHostStyleProvider/SandboxHostThemeProvider to assert a DOM-level signal that actually changes with the resolved host theme: render twice (once with SandboxHostThemeProvider value="dark" and once with value="light") and use screen.getByPlaceholderText("Type your message...") to verify the element's effective state toggles (for example by checking the element’s computed background style via getComputedStyle or by asserting a theme-specific attribute/class applied at runtime on the input or a known host-scoped container) so the test fails if theme resolution/regression occurs.
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/components/shared/__tests__/DisplayContextHeader.test.tsx (1)
68-161: Please use the shared client mock presets here.This suite recreates preference, playground, and client-config state by hand, which is easy to drift from the rest of the client harness. Switching these to the shared presets will keep the test aligned with the repository’s standard fixtures.
As per coding guidelines,
mcpjam-inspector/client/**/__tests__/*.test.{ts,tsx}: Use mock presets fromclient/src/test/mocks/includingmcpApiPresetsandstorePresetsin client tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/shared/__tests__/DisplayContextHeader.test.tsx` around lines 68 - 161, The test is hand-rolling preference, playground and client-config mocks (mockUpdateThemeMode, mockPreferencesState, mockUIPlaygroundStore, mockPatchHostContext and the vi.mock(...) blocks) instead of using the shared test presets; replace these custom mocks with the shared presets from client/src/test/mocks/ (e.g. import storePresets and mcpApiPresets) and wire the vi.mock implementations to return/select from those presets (use the preset objects for usePreferencesStore, useUIPlaygroundStore, useClientConfigStore and any theme-utils/client-config helpers) so the test uses the canonical fixtures used across the client test suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcpjam-inspector/client/src/components/chat-v2/shared/chat-helpers.ts`:
- Around line 95-133: The switch in getProviderColorForTheme accidentally
dropped the "openrouter" case and swapped colors for "moonshotai" and "z-ai"
compared to the canonical mappings, causing fallback/regression in badges;
update the getProviderColorForTheme function (and its inner resolveThemeClasses)
to add a case for "openrouter" with the correct light/dark classes and restore
the original color pairs for "moonshotai" and "z-ai" to match the mappings in
provider-logos.ts (use the same class strings there), ensuring all other
provider cases remain unchanged.
In
`@mcpjam-inspector/client/src/components/ui-playground/__tests__/PlaygroundMain.test.tsx`:
- Around line 300-309: Replace the inline mockPreferencesState and its vi.mock
implementation with the shared test presets: import storePresets (and
mcpApiPresets if needed) from the test/mocks module and have
vi.mock("@/stores/preferences/preferences-provider") return usePreferencesStore
that calls the selector against the appropriate preset (e.g., selector ?
selector(storePresets.preferences) : storePresets.preferences) instead of
mockPreferencesState; update any tests that referenced mockPreferencesState to
use the shared preset values from storePresets/mcpApiPresets and remove the
inline mock object.
In `@mcpjam-inspector/client/src/index.css`:
- Around line 111-139: The scoped theme class .app-theme-scope is missing the
--json-* CSS custom properties (e.g., --json-key, --json-string, --json-number,
--json-boolean, --json-null, etc.), causing JsonEditor instances to inherit the
global palette; add the same --json-* variable definitions currently on
:root/.dark into the .app-theme-scope block (and likewise into the scoped dark
variant around lines ~222-250) so the JsonEditor uses the local thread theme;
reference the .app-theme-scope selector and the --json-* token names when
applying the mirrored properties.
---
Duplicate comments:
In `@mcpjam-inspector/client/src/components/chat-v2/__tests__/ChatInput.test.tsx`:
- Around line 10-13: Replace the inline mock of usePreferencesStore in the test
with the shared test store presets: remove the vi.mock block that defines
usePreferencesStore and instead import and apply the existing storePresets (and
mcpApiPresets if used) from the test mocks in the test setup; ensure the test
uses storePresets to provide the themeMode ("light") value so references to
usePreferencesStore in the ChatInput tests resolve via the standard client test
harness rather than an inline mock.
- Around line 172-185: The test currently only checks for CSS utility class
names which can pass even if host-theme wiring breaks; update the test for
ChatInput inside SandboxHostStyleProvider/SandboxHostThemeProvider to assert a
DOM-level signal that actually changes with the resolved host theme: render
twice (once with SandboxHostThemeProvider value="dark" and once with
value="light") and use screen.getByPlaceholderText("Type your message...") to
verify the element's effective state toggles (for example by checking the
element’s computed background style via getComputedStyle or by asserting a
theme-specific attribute/class applied at runtime on the input or a known
host-scoped container) so the test fails if theme resolution/regression occurs.
In `@mcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsx`:
- Around line 806-840: When xrayMode is true the footer renders only ChatInput
so submit/auth errors (rendered by ErrorBox) are hidden; update the X-Ray footer
inside the StickToBottom block (where XRaySnapshotView and ChatInput are used)
to reuse the same composer + ErrorBox as the regular thread footer instead of
rendering only ChatInput. Locate the XRaySnapshotView/StickToBottom block and
replace the inner footer panel that currently renders ChatInput with the same
composer container used for threadContent (including ErrorBox and the composer
component referenced by sharedChatInputProps/ChatInput) or mirror ErrorBox into
that footer so submit and auth errors remain visible while xrayMode is active.
---
Nitpick comments:
In
`@mcpjam-inspector/client/src/components/shared/__tests__/DisplayContextHeader.test.tsx`:
- Around line 68-161: The test is hand-rolling preference, playground and
client-config mocks (mockUpdateThemeMode, mockPreferencesState,
mockUIPlaygroundStore, mockPatchHostContext and the vi.mock(...) blocks) instead
of using the shared test presets; replace these custom mocks with the shared
presets from client/src/test/mocks/ (e.g. import storePresets and mcpApiPresets)
and wire the vi.mock implementations to return/select from those presets (use
the preset objects for usePreferencesStore, useUIPlaygroundStore,
useClientConfigStore and any theme-utils/client-config helpers) so the test uses
the canonical fixtures used across the client test suite.
🪄 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: 38978c64-15d2-4def-baa6-b798e5931c95
📒 Files selected for processing (24)
mcpjam-inspector/client/src/components/chat-v2/__tests__/ChatInput.test.tsxmcpjam-inspector/client/src/components/chat-v2/__tests__/MessageView.test.tsxmcpjam-inspector/client/src/components/chat-v2/__tests__/ThinkingIndicator.test.tsxmcpjam-inspector/client/src/components/chat-v2/chat-input.tsxmcpjam-inspector/client/src/components/chat-v2/chat-input/model/provider-logo.tsxmcpjam-inspector/client/src/components/chat-v2/shared/assistant-avatar.tsmcpjam-inspector/client/src/components/chat-v2/shared/chat-helpers.tsmcpjam-inspector/client/src/components/chat-v2/shared/thinking-indicator.tsxmcpjam-inspector/client/src/components/chat-v2/thread/csp-debug-panel.tsxmcpjam-inspector/client/src/components/chat-v2/thread/message-view.tsxmcpjam-inspector/client/src/components/chat-v2/thread/parts/tool-part.tsxmcpjam-inspector/client/src/components/chat-v2/thread/user-message-bubble.tsxmcpjam-inspector/client/src/components/shared/DisplayContextHeader.tsxmcpjam-inspector/client/src/components/shared/__tests__/DisplayContextHeader.test.tsxmcpjam-inspector/client/src/components/sidebar/__tests__/nav-main.test.tsxmcpjam-inspector/client/src/components/ui-playground/PlaygroundLeft.tsxmcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsxmcpjam-inspector/client/src/components/ui-playground/TabHeader.tsxmcpjam-inspector/client/src/components/ui-playground/__tests__/PlaygroundMain.test.tsxmcpjam-inspector/client/src/contexts/sandbox-host-style-context.tsmcpjam-inspector/client/src/index.cssmcpjam-inspector/client/src/styles/presets/brutalist.cssmcpjam-inspector/client/src/styles/presets/soft-pop.cssmcpjam-inspector/client/src/styles/presets/tangerine.css
✅ Files skipped from review due to trivial changes (4)
- mcpjam-inspector/client/src/components/ui-playground/PlaygroundLeft.tsx
- mcpjam-inspector/client/src/components/chat-v2/tests/ThinkingIndicator.test.tsx
- mcpjam-inspector/client/src/styles/presets/tangerine.css
- mcpjam-inspector/client/src/components/chat-v2/chat-input.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- mcpjam-inspector/client/src/components/sidebar/tests/nav-main.test.tsx
- mcpjam-inspector/client/src/components/chat-v2/tests/MessageView.test.tsx
- mcpjam-inspector/client/src/styles/presets/brutalist.css
- mcpjam-inspector/client/src/components/chat-v2/chat-input/model/provider-logo.tsx
- mcpjam-inspector/client/src/components/ui-playground/TabHeader.tsx
- mcpjam-inspector/client/src/components/chat-v2/shared/thinking-indicator.tsx
- mcpjam-inspector/client/src/contexts/sandbox-host-style-context.ts
- mcpjam-inspector/client/src/styles/presets/soft-pop.css
- mcpjam-inspector/client/src/components/chat-v2/shared/assistant-avatar.ts
Comment on lines
+95
to
+133
| export const getProviderColorForTheme = ( | ||
| provider: string, | ||
| themeMode?: "light" | "dark" | "system", | ||
| ) => { | ||
| const resolveThemeClasses = (lightClasses: string, darkClasses: string) => { | ||
| if (themeMode === "light") return lightClasses; | ||
| if (themeMode === "dark") return darkClasses; | ||
| return `${lightClasses} dark:${darkClasses}`; | ||
| }; | ||
|
|
||
| switch (provider) { | ||
| case "anthropic": | ||
| return "text-orange-600 dark:text-orange-400"; | ||
| return resolveThemeClasses("text-orange-600", "text-orange-400"); | ||
| case "openai": | ||
| return "text-green-600 dark:text-green-400"; | ||
| return resolveThemeClasses("text-green-600", "text-green-400"); | ||
| case "deepseek": | ||
| return "text-blue-600 dark:text-blue-400"; | ||
| return resolveThemeClasses("text-blue-600", "text-blue-400"); | ||
| case "google": | ||
| return "text-red-600 dark:text-red-400"; | ||
| return resolveThemeClasses("text-red-600", "text-red-400"); | ||
| case "mistral": | ||
| return "text-orange-500 dark:text-orange-400"; | ||
| return resolveThemeClasses("text-orange-500", "text-orange-400"); | ||
| case "ollama": | ||
| return "text-gray-600 dark:text-gray-400"; | ||
| return resolveThemeClasses("text-gray-600", "text-gray-400"); | ||
| case "xai": | ||
| return "text-purple-600 dark:text-purple-400"; | ||
| return resolveThemeClasses("text-purple-600", "text-purple-400"); | ||
| case "azure": | ||
| return "text-purple-600 dark:text-purple-400"; | ||
| return resolveThemeClasses("text-purple-600", "text-purple-400"); | ||
| case "custom": | ||
| return "bg-gradient-to-br from-teal-500 to-cyan-600"; | ||
| case "moonshotai": | ||
| return "text-cyan-600 dark:text-cyan-400"; | ||
| return resolveThemeClasses("text-cyan-600", "text-cyan-400"); | ||
| case "z-ai": | ||
| return "text-indigo-600 dark:text-indigo-400"; | ||
| return resolveThemeClasses("text-indigo-600", "text-indigo-400"); | ||
| case "minimax": | ||
| return "text-pink-600 dark:text-pink-400"; | ||
| return resolveThemeClasses("text-pink-600", "text-pink-400"); | ||
| case "meta": | ||
| return "text-blue-500 dark:text-blue-400"; | ||
| return resolveThemeClasses("text-blue-500", "text-blue-400"); | ||
| default: | ||
| return "text-blue-600 dark:text-blue-400"; | ||
| return resolveThemeClasses("text-blue-600", "text-blue-400"); |
Contributor
There was a problem hiding this comment.
Restore the dropped provider color mappings.
This switch no longer handles openrouter, and it swaps the old moonshotai / z-ai colors from mcpjam-inspector/client/src/lib/provider-logos.ts:92-123. Fallback badges will quietly regress.
Patch sketch
case "custom":
return "bg-gradient-to-br from-teal-500 to-cyan-600";
+ case "openrouter":
+ return resolveThemeClasses("text-purple-500", "text-purple-400");
case "moonshotai":
- return resolveThemeClasses("text-cyan-600", "text-cyan-400");
+ return resolveThemeClasses("text-indigo-600", "text-indigo-400");
case "z-ai":
- return resolveThemeClasses("text-indigo-600", "text-indigo-400");
+ return resolveThemeClasses("text-cyan-600", "text-cyan-400");📝 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.
Suggested change
| export const getProviderColorForTheme = ( | |
| provider: string, | |
| themeMode?: "light" | "dark" | "system", | |
| ) => { | |
| const resolveThemeClasses = (lightClasses: string, darkClasses: string) => { | |
| if (themeMode === "light") return lightClasses; | |
| if (themeMode === "dark") return darkClasses; | |
| return `${lightClasses} dark:${darkClasses}`; | |
| }; | |
| switch (provider) { | |
| case "anthropic": | |
| return "text-orange-600 dark:text-orange-400"; | |
| return resolveThemeClasses("text-orange-600", "text-orange-400"); | |
| case "openai": | |
| return "text-green-600 dark:text-green-400"; | |
| return resolveThemeClasses("text-green-600", "text-green-400"); | |
| case "deepseek": | |
| return "text-blue-600 dark:text-blue-400"; | |
| return resolveThemeClasses("text-blue-600", "text-blue-400"); | |
| case "google": | |
| return "text-red-600 dark:text-red-400"; | |
| return resolveThemeClasses("text-red-600", "text-red-400"); | |
| case "mistral": | |
| return "text-orange-500 dark:text-orange-400"; | |
| return resolveThemeClasses("text-orange-500", "text-orange-400"); | |
| case "ollama": | |
| return "text-gray-600 dark:text-gray-400"; | |
| return resolveThemeClasses("text-gray-600", "text-gray-400"); | |
| case "xai": | |
| return "text-purple-600 dark:text-purple-400"; | |
| return resolveThemeClasses("text-purple-600", "text-purple-400"); | |
| case "azure": | |
| return "text-purple-600 dark:text-purple-400"; | |
| return resolveThemeClasses("text-purple-600", "text-purple-400"); | |
| case "custom": | |
| return "bg-gradient-to-br from-teal-500 to-cyan-600"; | |
| case "moonshotai": | |
| return "text-cyan-600 dark:text-cyan-400"; | |
| return resolveThemeClasses("text-cyan-600", "text-cyan-400"); | |
| case "z-ai": | |
| return "text-indigo-600 dark:text-indigo-400"; | |
| return resolveThemeClasses("text-indigo-600", "text-indigo-400"); | |
| case "minimax": | |
| return "text-pink-600 dark:text-pink-400"; | |
| return resolveThemeClasses("text-pink-600", "text-pink-400"); | |
| case "meta": | |
| return "text-blue-500 dark:text-blue-400"; | |
| return resolveThemeClasses("text-blue-500", "text-blue-400"); | |
| default: | |
| return "text-blue-600 dark:text-blue-400"; | |
| return resolveThemeClasses("text-blue-600", "text-blue-400"); | |
| export const getProviderColorForTheme = ( | |
| provider: string, | |
| themeMode?: "light" | "dark" | "system", | |
| ) => { | |
| const resolveThemeClasses = (lightClasses: string, darkClasses: string) => { | |
| if (themeMode === "light") return lightClasses; | |
| if (themeMode === "dark") return darkClasses; | |
| return `${lightClasses} dark:${darkClasses}`; | |
| }; | |
| switch (provider) { | |
| case "anthropic": | |
| return resolveThemeClasses("text-orange-600", "text-orange-400"); | |
| case "openai": | |
| return resolveThemeClasses("text-green-600", "text-green-400"); | |
| case "deepseek": | |
| return resolveThemeClasses("text-blue-600", "text-blue-400"); | |
| case "google": | |
| return resolveThemeClasses("text-red-600", "text-red-400"); | |
| case "mistral": | |
| return resolveThemeClasses("text-orange-500", "text-orange-400"); | |
| case "ollama": | |
| return resolveThemeClasses("text-gray-600", "text-gray-400"); | |
| case "xai": | |
| return resolveThemeClasses("text-purple-600", "text-purple-400"); | |
| case "azure": | |
| return resolveThemeClasses("text-purple-600", "text-purple-400"); | |
| case "custom": | |
| return "bg-gradient-to-br from-teal-500 to-cyan-600"; | |
| case "openrouter": | |
| return resolveThemeClasses("text-purple-500", "text-purple-400"); | |
| case "moonshotai": | |
| return resolveThemeClasses("text-indigo-600", "text-indigo-400"); | |
| case "z-ai": | |
| return resolveThemeClasses("text-cyan-600", "text-cyan-400"); | |
| case "minimax": | |
| return resolveThemeClasses("text-pink-600", "text-pink-400"); | |
| case "meta": | |
| return resolveThemeClasses("text-blue-500", "text-blue-400"); | |
| default: | |
| return resolveThemeClasses("text-blue-600", "text-blue-400"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/components/chat-v2/shared/chat-helpers.ts` around
lines 95 - 133, The switch in getProviderColorForTheme accidentally dropped the
"openrouter" case and swapped colors for "moonshotai" and "z-ai" compared to the
canonical mappings, causing fallback/regression in badges; update the
getProviderColorForTheme function (and its inner resolveThemeClasses) to add a
case for "openrouter" with the correct light/dark classes and restore the
original color pairs for "moonshotai" and "z-ai" to match the mappings in
provider-logos.ts (use the same class strings there), ensuring all other
provider cases remain unchanged.
Comment on lines
+300
to
309
| const mockPreferencesState = { | ||
| themeMode: "light", | ||
| themePreset: "soft-pop", | ||
| setThemeMode: vi.fn(), | ||
| }; | ||
|
|
||
| vi.mock("@/stores/preferences/preferences-provider", () => ({ | ||
| usePreferencesStore: (selector: any) => { | ||
| const state = { | ||
| themeMode: "light", | ||
| setThemeMode: vi.fn(), | ||
| }; | ||
| return selector ? selector(state) : state; | ||
| }, | ||
| usePreferencesStore: (selector: any) => | ||
| selector ? selector(mockPreferencesState) : mockPreferencesState, | ||
| })); |
Contributor
There was a problem hiding this comment.
Switch this preferences mock to shared storePresets/mcpApiPresets.
This inline store mock diverges from the client test harness pattern and makes test state shape easier to desynchronize over time.
As per coding guidelines, mcpjam-inspector/client/**/__tests__/*.test.{ts,tsx}: Use mock presets from client/src/test/mocks/ including mcpApiPresets and storePresets in client tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@mcpjam-inspector/client/src/components/ui-playground/__tests__/PlaygroundMain.test.tsx`
around lines 300 - 309, Replace the inline mockPreferencesState and its vi.mock
implementation with the shared test presets: import storePresets (and
mcpApiPresets if needed) from the test/mocks module and have
vi.mock("@/stores/preferences/preferences-provider") return usePreferencesStore
that calls the selector against the appropriate preset (e.g., selector ?
selector(storePresets.preferences) : storePresets.preferences) instead of
mockPreferencesState; update any tests that referenced mockPreferencesState to
use the shared preset values from storePresets/mcpApiPresets and remove the
inline mock object.
Comment on lines
+111
to
+139
| .app-theme-scope { | ||
| --background: oklch(0.9818 0.0054 95.0986); | ||
| --foreground: oklch(0.3438 0.0269 95.7226); | ||
| --card: oklch(0.9818 0.0054 95.0986); | ||
| --card-foreground: oklch(0.1908 0.002 106.5859); | ||
| --popover: oklch(1 0 0); | ||
| --popover-foreground: oklch(0.2671 0.0196 98.939); | ||
| --primary: oklch(0.6832 0.1382 38.744); | ||
| --primary-foreground: oklch(1 0 0); | ||
| --secondary: oklch(0.9245 0.0138 92.9892); | ||
| --secondary-foreground: oklch(0.4334 0.0177 98.6048); | ||
| --muted: oklch(0.9341 0.0153 90.239); | ||
| --muted-foreground: oklch(0.6059 0.0075 97.4233); | ||
| --accent: oklch(0.9245 0.0138 92.9892); | ||
| --accent-foreground: oklch(0.2671 0.0196 98.939); | ||
| --destructive: oklch(0.55 0.22 27); | ||
| --destructive-foreground: oklch(0.25 0.1 25); | ||
| --border: oklch(0.8847 0.0069 97.3627); | ||
| --input: oklch(0.7621 0.0156 98.3528); | ||
| --ring: oklch(0.6171 0.1375 39.0427); | ||
| --success: oklch(0.696 0.17 152.5); | ||
| --success-foreground: oklch(1 0 0); | ||
| --warning: oklch(0.769 0.188 85.3); | ||
| --warning-foreground: oklch(0.239 0.06 60); | ||
| --info: oklch(0.623 0.214 259); | ||
| --info-foreground: oklch(1 0 0); | ||
| --pending: oklch(0.769 0.188 85.3); | ||
| --pending-foreground: oklch(0.239 0.06 60); | ||
| } |
Contributor
There was a problem hiding this comment.
The scoped theme is still missing the JSON palette.
.json-* classes later in this file read --json-key, --json-string, and the rest of the --json-* vars, but those tokens are only defined on :root / .dark, not on .app-theme-scope. Any JsonEditor rendered inside the sandbox will therefore keep inheriting the outer app palette whenever the local thread theme differs from the global one.
Please mirror the --json-* vars into both scoped blocks, or the thread theme isolation stays incomplete.
Also applies to: 222-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/index.css` around lines 111 - 139, The scoped
theme class .app-theme-scope is missing the --json-* CSS custom properties
(e.g., --json-key, --json-string, --json-number, --json-boolean, --json-null,
etc.), causing JsonEditor instances to inherit the global palette; add the same
--json-* variable definitions currently on :root/.dark into the .app-theme-scope
block (and likewise into the scoped dark variant around lines ~222-250) so the
JsonEditor uses the local thread theme; reference the .app-theme-scope selector
and the --json-* token names when applying the mirrored properties.
chelojimenez
approved these changes
Mar 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(app-builder): restore host thread visuals with local theme scoping
before:
Screen.Recording.2026-03-27.at.2.59.09.PM.mov
Screen.Recording.2026-03-27.at.2.57.05.PM.mov
after:
Screen.Recording.2026-03-27.at.2.53.24.PM.mov
Screen.Recording.2026-03-27.at.2.54.45.PM.mov
also, nit in app builder:
before:
after: