Clarify swarm insights rail and fix environment list crash - #3759
Clarify swarm insights rail and fix environment list crash#3759chelojimenez wants to merge 1 commit into
Conversation
Separate struggle patterns from rubric checks with plainer headers, hide resolved signals by default, and soften the scorecard so it cannot read as an overall grade. Stop passing origin to listEnvironments until the backend accepts it, fixing the Convex validation crash on older deploys. Co-authored-by: Cursor <cursoragent@cursor.com>
dosubot
Bot
added
the
size:L
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e18e154f-9fe6-4dc4-a658-fd4754312c2f) |
✅ 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. |
WalkthroughThe PR redesigns swarm insights cards around clean-criteria counts and criterion-level states instead of percentage grades. It adds resolved-finding filtering, updated panel layout and controls, revised spend-cap messaging, and coverage for these behaviors. It also adjusts insights rail sizing and removes the 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mcpjam-inspector/client/src/components/swarms/CriterionScorecard.tsx`:
- Line 59: Update the allClean calculation in CriterionScorecard so the footer
reports checks passed only when every criterion is graded and clean, not merely
when totalFail is zero; align it with the header’s 1/2 clean state and add a
regression test covering one passing criterion plus one entirely ungraded
criterion.
🪄 Autofix
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 Plus
Run ID: 301b4f1d-760a-47c7-a64b-9beafe5b4108
📒 Files selected for processing (8)
mcpjam-inspector/client/src/components/swarms/CriterionScorecard.tsxmcpjam-inspector/client/src/components/swarms/SwarmInsightsPanel.tsxmcpjam-inspector/client/src/components/swarms/__tests__/SwarmInsightsPanel.criteria.test.tsxmcpjam-inspector/client/src/components/swarms/__tests__/SwarmInsightsPanel.test.tsxmcpjam-inspector/client/src/components/swarms/__tests__/swarm-run-insights.test.tsxmcpjam-inspector/client/src/components/swarms/swarm-run-insights.tsxmcpjam-inspector/client/src/hooks/use-swarm-run-insights.tsmcpjam-inspector/client/src/hooks/useProjectEnvironments.ts
| const cleanCount = facets.filter( | ||
| (f) => f.passCount + f.failCount > 0 && f.failCount === 0, | ||
| ).length; | ||
| const allClean = totalGraded > 0 && totalFail === 0; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not report all checks as passed when a criterion is ungraded.
Line 59 treats any zero-failure aggregate as clean. If one criterion passed and another has only ungraded sessions, the footer renders “Checks passed” although the header correctly shows 1/2 clean.
Require every criterion to be clean before rendering the bridge. Add a mixed pass/ungraded regression test.
Proposed fix
- const allClean = totalGraded > 0 && totalFail === 0;
+ const allClean = cleanCount === facets.length;📝 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.
| const allClean = totalGraded > 0 && totalFail === 0; | |
| const allClean = cleanCount === facets.length; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mcpjam-inspector/client/src/components/swarms/CriterionScorecard.tsx` at line
59, Update the allClean calculation in CriterionScorecard so the footer reports
checks passed only when every criterion is graded and clean, not merely when
totalFail is zero; align it with the header’s 1/2 clean state and add a
regression test covering one passing criterion plus one entirely ungraded
criterion.
Internal previewPreview URL: https://mcp-inspector-pr-3759.up.railway.app |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="mcpjam-inspector/client/src/components/swarms/CriterionScorecard.tsx">
<violation number="1" location="mcpjam-inspector/client/src/components/swarms/CriterionScorecard.tsx:59">
P2: The scorecard can claim “Checks passed” while a configured criterion has no completed grades; derive this bridge from the same per-criterion clean definition used by `cleanCount`, so partially graded rubrics do not look fully passed.</violation>
</file>
<file name="mcpjam-inspector/client/src/hooks/useProjectEnvironments.ts">
<violation number="1" location="mcpjam-inspector/client/src/hooks/useProjectEnvironments.ts:119">
P1: The `includeAdhoc` opt-in no longer works on supported backends: omitting `origin: "all"` selects the named-only backend default, so selected ad-hoc environments become unavailable for labeling and the management route cannot browse or name them. Retain the opt-in for compatible deploys and add a compatibility strategy for older deployments instead of removing it unconditionally.</violation>
</file>
<file name="mcpjam-inspector/client/src/components/swarms/swarm-run-insights.tsx">
<violation number="1" location="mcpjam-inspector/client/src/components/swarms/swarm-run-insights.tsx:177">
P2: The resolved filter is not defaulted per run: after a user clicks “Show resolved” on one run, navigating directly to another run in the same project keeps `showResolved` true and exposes resolved rows there. Reset this state when `swarmRunGroupId` changes (or key the insights component by the run id).</violation>
<violation number="2" location="mcpjam-inspector/client/src/components/swarms/swarm-run-insights.tsx:474">
P3: On touch-sized screens, open findings can have an invisible Dismiss button with no discoverable way to reveal it. Keep this action visible below the hover-capable breakpoint, while retaining hover/focus reveal on desktop.</violation>
</file>
<file name="mcpjam-inspector/client/src/components/swarms/__tests__/swarm-run-insights.test.tsx">
<violation number="1" location="mcpjam-inspector/client/src/components/swarms/__tests__/swarm-run-insights.test.tsx:336">
P3: The mock `errorMessage` here is decorative: for `errorCode: "spend_cap_exceeded"` the hook ignores `dto.errorMessage` and hardcodes the same text in `use-swarm-run-insights.ts`, so this test would keep passing even if the field were removed. Prefer driving the string through the hook path you intend to cover (e.g. use a non-hardcoded errorCode) so the fixture actually exercises the code being asserted, or drop the redundant field.</violation>
</file>
<file name="mcpjam-inspector/client/src/components/swarms/SwarmInsightsPanel.tsx">
<violation number="1" location="mcpjam-inspector/client/src/components/swarms/SwarmInsightsPanel.tsx:385">
P2: Changing the findings and scorecard columns from overflow-y-auto to overflow-hidden removes their independent scrolling, and because both are flex-1 + min-h-0 children sized to the rail's available height, the rail's own overflow-y-auto never engages either — so content taller than a column (a long findings list or a scorecard with many criteria, at a 45% rail cap) is silently clipped with no way to reach it. Consider keeping overflow-y-auto on the columns, or letting the rail scroll by allowing the columns to exceed its height (drop min-h-0/flex-1 constraint) instead of overflow-hidden.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * explicitly would turn a harmless skew into a hard failure. | ||
| * Do NOT send `origin` until every deployed backend accepts it — Convex | ||
| * rejects unknown args and the whole surface crashes. When `includeAdhoc` is | ||
| * true against an older backend, ad-hoc rows simply won't appear in the |
There was a problem hiding this comment.
P1: The includeAdhoc opt-in no longer works on supported backends: omitting origin: "all" selects the named-only backend default, so selected ad-hoc environments become unavailable for labeling and the management route cannot browse or name them. Retain the opt-in for compatible deploys and add a compatibility strategy for older deployments instead of removing it unconditionally.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/hooks/useProjectEnvironments.ts, line 119:
<comment>The `includeAdhoc` opt-in no longer works on supported backends: omitting `origin: "all"` selects the named-only backend default, so selected ad-hoc environments become unavailable for labeling and the management route cannot browse or name them. Retain the opt-in for compatible deploys and add a compatibility strategy for older deployments instead of removing it unconditionally.</comment>
<file context>
@@ -114,9 +114,10 @@ export interface ProjectEnvironmentView {
- * explicitly would turn a harmless skew into a hard failure.
+ * Do NOT send `origin` until every deployed backend accepts it — Convex
+ * rejects unknown args and the whole surface crashes. When `includeAdhoc` is
+ * true against an older backend, ad-hoc rows simply won't appear in the
+ * response; the client-side filter below still applies once they do.
*/
</file context>
| const cleanCount = facets.filter( | ||
| (f) => f.passCount + f.failCount > 0 && f.failCount === 0, | ||
| ).length; | ||
| const allClean = totalGraded > 0 && totalFail === 0; |
There was a problem hiding this comment.
P2: The scorecard can claim “Checks passed” while a configured criterion has no completed grades; derive this bridge from the same per-criterion clean definition used by cleanCount, so partially graded rubrics do not look fully passed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/components/swarms/CriterionScorecard.tsx, line 59:
<comment>The scorecard can claim “Checks passed” while a configured criterion has no completed grades; derive this bridge from the same per-criterion clean definition used by `cleanCount`, so partially graded rubrics do not look fully passed.</comment>
<file context>
@@ -53,141 +50,151 @@ export function CriterionScorecard({
const cleanCount = facets.filter(
(f) => f.passCount + f.failCount > 0 && f.failCount === 0,
).length;
+ const allClean = totalGraded > 0 && totalFail === 0;
return (
</file context>
| const allClean = totalGraded > 0 && totalFail === 0; | |
| const allClean = totalGraded > 0 && cleanCount === facets.length; |
| const [showAll, setShowAll] = useState(false); | ||
| // Resolved registry rows are history — default to open problems so the | ||
| // panel answers "what still matters" instead of replaying green chips. | ||
| const [showResolved, setShowResolved] = useState(false); |
There was a problem hiding this comment.
P2: The resolved filter is not defaulted per run: after a user clicks “Show resolved” on one run, navigating directly to another run in the same project keeps showResolved true and exposes resolved rows there. Reset this state when swarmRunGroupId changes (or key the insights component by the run id).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/components/swarms/swarm-run-insights.tsx, line 177:
<comment>The resolved filter is not defaulted per run: after a user clicks “Show resolved” on one run, navigating directly to another run in the same project keeps `showResolved` true and exposes resolved rows there. Reset this state when `swarmRunGroupId` changes (or key the insights component by the run id).</comment>
<file context>
@@ -172,6 +172,9 @@ export function SwarmRunInsights({
const [showAll, setShowAll] = useState(false);
+ // Resolved registry rows are history — default to open problems so the
+ // panel answers "what still matters" instead of replaying green chips.
+ const [showResolved, setShowResolved] = useState(false);
const rows: Row[] = useMemo(() => {
</file context>
| <> | ||
| {children ? ( | ||
| <div className="min-h-0 min-w-0 flex-1 overflow-y-auto"> | ||
| <div className="flex min-h-0 min-w-0 flex-1 flex-col overflow-hidden"> |
There was a problem hiding this comment.
P2: Changing the findings and scorecard columns from overflow-y-auto to overflow-hidden removes their independent scrolling, and because both are flex-1 + min-h-0 children sized to the rail's available height, the rail's own overflow-y-auto never engages either — so content taller than a column (a long findings list or a scorecard with many criteria, at a 45% rail cap) is silently clipped with no way to reach it. Consider keeping overflow-y-auto on the columns, or letting the rail scroll by allowing the columns to exceed its height (drop min-h-0/flex-1 constraint) instead of overflow-hidden.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/components/swarms/SwarmInsightsPanel.tsx, line 385:
<comment>Changing the findings and scorecard columns from overflow-y-auto to overflow-hidden removes their independent scrolling, and because both are flex-1 + min-h-0 children sized to the rail's available height, the rail's own overflow-y-auto never engages either — so content taller than a column (a long findings list or a scorecard with many criteria, at a 45% rail cap) is silently clipped with no way to reach it. Consider keeping overflow-y-auto on the columns, or letting the rail scroll by allowing the columns to exceed its height (drop min-h-0/flex-1 constraint) instead of overflow-hidden.</comment>
<file context>
@@ -382,11 +382,11 @@ export function SwarmInsightsPanel({
<>
{children ? (
- <div className="min-h-0 min-w-0 flex-1 overflow-y-auto">
+ <div className="flex min-h-0 min-w-0 flex-1 flex-col overflow-hidden">
{children}
</div>
</file context>
| // a dismissed finding is always recoverable. | ||
| dismissed | ||
| ? "opacity-100" | ||
| : "opacity-0 focus-visible:opacity-100 group-hover:opacity-100", |
There was a problem hiding this comment.
P3: On touch-sized screens, open findings can have an invisible Dismiss button with no discoverable way to reveal it. Keep this action visible below the hover-capable breakpoint, while retaining hover/focus reveal on desktop.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/components/swarms/swarm-run-insights.tsx, line 474:
<comment>On touch-sized screens, open findings can have an invisible Dismiss button with no discoverable way to reveal it. Keep this action visible below the hover-capable breakpoint, while retaining hover/focus reveal on desktop.</comment>
<file context>
@@ -419,7 +465,14 @@ function InsightRow({
+ // a dismissed finding is always recoverable.
+ dismissed
+ ? "opacity-100"
+ : "opacity-0 focus-visible:opacity-100 group-hover:opacity-100",
+ )}
onClick={toggleDismiss}
</file context>
| : "opacity-0 focus-visible:opacity-100 group-hover:opacity-100", | |
| : "opacity-100 sm:opacity-0 sm:focus-visible:opacity-100 sm:group-hover:opacity-100", |
| discovery: null, | ||
| errorCode: "spend_cap_exceeded", | ||
| errorMessage: "Spending cap reached — insights were not generated.", | ||
| errorMessage: "Explanation unavailable — spend cap.", |
There was a problem hiding this comment.
P3: The mock errorMessage here is decorative: for errorCode: "spend_cap_exceeded" the hook ignores dto.errorMessage and hardcodes the same text in use-swarm-run-insights.ts, so this test would keep passing even if the field were removed. Prefer driving the string through the hook path you intend to cover (e.g. use a non-hardcoded errorCode) so the fixture actually exercises the code being asserted, or drop the redundant field.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/client/src/components/swarms/__tests__/swarm-run-insights.test.tsx, line 336:
<comment>The mock `errorMessage` here is decorative: for `errorCode: "spend_cap_exceeded"` the hook ignores `dto.errorMessage` and hardcodes the same text in `use-swarm-run-insights.ts`, so this test would keep passing even if the field were removed. Prefer driving the string through the hook path you intend to cover (e.g. use a non-hardcoded errorCode) so the fixture actually exercises the code being asserted, or drop the redundant field.</comment>
<file context>
@@ -333,12 +333,13 @@ describe("generation lifecycle", () => {
discovery: null,
errorCode: "spend_cap_exceeded",
- errorMessage: "Spending cap reached — insights were not generated.",
+ errorMessage: "Explanation unavailable — spend cap.",
updatedAt: 1,
};
</file context>
1 participant
Summary
originargument toprojectEnvironments:listEnvironments, which was crashing Swarm run detail against older Convex deploys.Test plan
swarm-run-insights.test.tsx,SwarmInsightsPanel.criteria.test.tsx, andSwarmInsightsPanel.test.tsxpass locallyoriginarg and confirm the page loads without the Convex validation errorMade with Cursor
Summary by cubic
Clarifies the Swarm run Insights rail for clearer signals and hides noise by default; also removes an unsupported
originarg to fix environment list crashes on older Convex backends.New Features
Bug Fixes
origintoprojectEnvironments:listEnvironments, preventing Convex validation crashes on older deployments.Written for commit e17aab9. Summary will update on new commits.