Evals playground information architecture#2271
Conversation
✅ 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. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
dosubot
Bot
added
size:L
prathmeshpatel
changed the title
Internal previewPreview URL: https://mcp-inspector-pr-2271.up.railway.app |
WalkthroughThis PR introduces suite workspace navigation with a tabbed interface for dashboard, runs, test-cases, and settings views. It adds a suite dashboard data assembly layer that derives the latest completed run, aggregates case and model identifiers, and computes available metrics based on iteration signals. The changes integrate workspace navigation into the suite overview page, add an IA surface dependency model, centralize route view handling via shared constants, and disable the global host bar when on the evals playground. Comprehensive tests verify route parsing, workspace section mapping, dashboard data assembly, and view behavior across different navigation contexts. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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/evals/eval-ia-surfaces.ts`:
- Around line 56-60: The current getEvalIaSurfaceReference uses
EVAL_IA_SURFACE_SEQUENCE.find(... )! which relies on a non-null assertion;
instead build a Record<EvalIaSurfaceId, EvalIaSurfaceReference> (e.g.,
SURFACE_BY_ID) from EVAL_IA_SURFACE_SEQUENCE at module initialization,
validating at runtime that every EvalIaSurfaceId appears exactly once (throw an
Error if a missing or duplicate id is detected), and then have
getEvalIaSurfaceReference(id: EvalIaSurfaceId) simply return SURFACE_BY_ID[id]
without using !; reference EVAL_IA_SURFACE_SEQUENCE, EvalIaSurfaceId,
EvalIaSurfaceReference and the getEvalIaSurfaceReference function when making
the change.
🪄 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: a0d12e49-89d8-4483-8c01-389adaf60b10
📒 Files selected for processing (15)
mcpjam-inspector/client/src/App.tsxmcpjam-inspector/client/src/__tests__/App.hosted-oauth.test.tsxmcpjam-inspector/client/src/components/evals/__tests__/create-suite-navigation.test.tsmcpjam-inspector/client/src/components/evals/__tests__/eval-ia-surfaces.test.tsmcpjam-inspector/client/src/components/evals/__tests__/suite-dashboard-data.test.tsmcpjam-inspector/client/src/components/evals/__tests__/suite-iterations-master-detail.test.tsxmcpjam-inspector/client/src/components/evals/eval-ia-surfaces.tsmcpjam-inspector/client/src/components/evals/suite-dashboard-data.tsmcpjam-inspector/client/src/components/evals/suite-iterations-view.tsxmcpjam-inspector/client/src/components/evals/suite-workspace-nav.tsxmcpjam-inspector/client/src/lib/__tests__/eval-route-url.test.tsmcpjam-inspector/client/src/lib/__tests__/eval-suite-ia.test.tsmcpjam-inspector/client/src/lib/app-navigation.tsmcpjam-inspector/client/src/lib/eval-route-url.tsmcpjam-inspector/client/src/lib/eval-suite-ia.ts
| export function getEvalIaSurfaceReference( | ||
| id: EvalIaSurfaceId, | ||
| ): EvalIaSurfaceReference { | ||
| return EVAL_IA_SURFACE_SEQUENCE.find((surface) => surface.id === id)!; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify there are no unsafe/dynamic call paths and that ids in sequence are unique.
set -euo pipefail
echo "Usages of getEvalIaSurfaceReference:"
rg -nP '\bgetEvalIaSurfaceReference\s*\(' --type ts --type tsx -C2
echo
echo "Check sequence id literals and duplicates in eval-ia-surfaces.ts:"
python - <<'PY'
import re, pathlib
p = pathlib.Path("mcpjam-inspector/client/src/components/evals/eval-ia-surfaces.ts")
s = p.read_text()
ids = re.findall(r'id:\s*"([^"]+)"', s)
print("ids:", ids)
print("unique:", sorted(set(ids)))
print("duplicate_count:", len(ids) - len(set(ids)))
PYRepository: MCPJam/inspector
Length of output: 143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="mcpjam-inspector/client/src/components/evals/eval-ia-surfaces.ts"
echo "== getEvalIaSurfaceReference call sites =="
rg -n --glob '*.ts' --glob '*.tsx' -C2 '\bgetEvalIaSurfaceReference\s*\(' .
echo
echo "== Inspect EVAL_IA_SURFACE_SEQUENCE + getEvalIaSurfaceReference =="
nl -ba "$FILE" | sed -n '1,140p'
echo
echo "== Parse IDs from EVAL_IA_SURFACE_SEQUENCE for duplicates =="
python3 - <<'PY'
import re, pathlib
p = pathlib.Path("mcpjam-inspector/client/src/components/evals/eval-ia-surfaces.ts")
s = p.read_text()
# grab id: "..." literals inside the sequence array (best-effort)
ids = re.findall(r'EVAL_IA_SURFACE_SEQUENCE\s*=\s*\[\s*[\s\S]*?id:\s*"([^"]+)"', s)
if not ids:
ids = re.findall(r'\bid:\s*"([^"]+)"', s)
print("ids:", ids)
print("unique:", sorted(set(ids)))
print("duplicate_count:", len(ids) - len(set(ids)))
PY
echo
echo "== Find EvalIaSurfaceId type definition =="
rg -n --glob '*.ts' --glob '*.tsx' 'type\s+EvalIaSurfaceId|enum\s+EvalIaSurfaceId|export\s+type\s+EvalIaSurfaceId|export\s+type\s+.*EvalIaSurfaceId' mcpjam-inspector/client/src/components/evals -S || trueRepository: MCPJam/inspector
Length of output: 1587
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="mcpjam-inspector/client/src/components/evals/eval-ia-surfaces.ts"
echo "== File excerpt (sequence + helper) =="
sed -n '1,130p' "$FILE"
echo
echo "== Evaluate getEvalIaSurfaceReference call sites (ts/tsx) =="
rg -n --glob '*.ts' --glob '*.tsx' '\bgetEvalIaSurfaceReference\s*\(' mcpjam-inspector/client/src/components/evals
echo
echo "== Inspect unit tests for this helper =="
sed -n '1,120p' mcpjam-inspector/client/src/components/evals/__tests__/eval-ia-surfaces.test.ts
echo
echo "== Extract EvalIaSurfaceId definition =="
rg -n --glob '*.ts' --glob '*.tsx' 'EvalIaSurfaceId' mcpjam-inspector/client/src/components/evals/eval-ia-surfaces.tsRepository: MCPJam/inspector
Length of output: 2894
Replace non-null assertion in getEvalIaSurfaceReference with a total keyed lookup (EvalIaSurfaceId).
mcpjam-inspector/client/src/components/evals/eval-ia-surfaces.ts:56-60 currently works because EvalIaSurfaceId is exactly "S1" | ... | "S6" and EVAL_IA_SURFACE_SEQUENCE includes all six ids (covered by tests). Still, EVAL_IA_SURFACE_SEQUENCE.find(... )! doesn’t guarantee completeness/uniqueness, so any future edit that drops or duplicates an id can yield undefined and crash at runtime.
- Replace it with a
Record<EvalIaSurfaceId, EvalIaSurfaceReference>derived fromEVAL_IA_SURFACE_SEQUENCE(with a small runtime invariant for completeness/uniqueness), and return the map value—no!.
🤖 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/evals/eval-ia-surfaces.ts` around
lines 56 - 60, The current getEvalIaSurfaceReference uses
EVAL_IA_SURFACE_SEQUENCE.find(... )! which relies on a non-null assertion;
instead build a Record<EvalIaSurfaceId, EvalIaSurfaceReference> (e.g.,
SURFACE_BY_ID) from EVAL_IA_SURFACE_SEQUENCE at module initialization,
validating at runtime that every EvalIaSurfaceId appears exactly once (throw an
Error if a missing or duplicate id is detected), and then have
getEvalIaSurfaceReference(id: EvalIaSurfaceId) simply return SURFACE_BY_ID[id]
without using !; reference EVAL_IA_SURFACE_SEQUENCE, EvalIaSurfaceId,
EvalIaSurfaceReference and the getEvalIaSurfaceReference function when making
the change.
2 participants
/evals/suite/:suiteIdroute-compatible as the Dashboard default