fix(studio): stop evaluation fileset links 404ing by default - #1199
Conversation
The Eval Config, Artifacts, and Dataset links on the evaluation pages all pointed at the unified fileset detail page via getFilesetDetailRoute. That route is registered only when FILESET_DETAILS_ENABLED is on, and the flag defaults to false, so on a default deployment every one of those links landed on a 404. Add getFilesetRoute, which picks the route that is actually registered and normalizes the reference for it: the detail page takes a bare name and derives the namespace itself, while the panel route needs a full namespace/name entity reference. Point the evaluation links at it. FilesetListRoute and useCreateFileset already guarded on the flag and are unchanged. FilesetCreateModal calls getFilesetDetailRoute directly, but both of its entry points are themselves flag-gated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Henrique Tolentino <htolentino@nvidia.com>
📝 WalkthroughWalkthroughThe PR adds ChangesFileset routing
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/packages/studio/src/routes/utils.test.ts (1)
70-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the enabled route branch.
These tests assert only the legacy panel URL. Add controlled tests with
FILESET_DETAILS_ENABLEDenabled for bare and namespaced references, includingoptions.tab. This protects the detail route and its reference normalization.🤖 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 `@web/packages/studio/src/routes/utils.test.ts` around lines 70 - 84, Extend the getFilesetRoute tests to cover the FILESET_DETAILS_ENABLED-enabled branch for both bare and already-namespaced references, including an options.tab value. Assert the detail-route URL and normalized entity references while preserving the existing legacy-route coverage.web/packages/studio/src/routes/utils.ts (1)
491-495: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the exported route contract explicit.
getFilesetRouterepeats the inline{ tab?: FilesetDetailTab }shape and infers its public return type. Define oneFilesetRouteOptionsinterface, reuse it in both route helpers, marktabasreadonly, and declare the return type asstring.Proposed refactor
+interface FilesetRouteOptions { + readonly tab?: FilesetDetailTab; +} ... - options?: { tab?: FilesetDetailTab } + options?: FilesetRouteOptions -) => { +): string => {As per coding guidelines, prefer
interfacefor object shapes and contracts, usereadonlyfor immutable properties, and use explicit return types for public APIs.🤖 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 `@web/packages/studio/src/routes/utils.ts` around lines 491 - 495, Update getFilesetRoute and the other fileset route helper to use a shared exported FilesetRouteOptions interface instead of inline option shapes; define its tab property as readonly and optional, and explicitly declare getFilesetRoute’s public return type as string.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@web/packages/studio/src/routes/utils.test.ts`:
- Around line 70-84: Extend the getFilesetRoute tests to cover the
FILESET_DETAILS_ENABLED-enabled branch for both bare and already-namespaced
references, including an options.tab value. Assert the detail-route URL and
normalized entity references while preserving the existing legacy-route
coverage.
In `@web/packages/studio/src/routes/utils.ts`:
- Around line 491-495: Update getFilesetRoute and the other fileset route helper
to use a shared exported FilesetRouteOptions interface instead of inline option
shapes; define its tab property as readonly and optional, and explicitly declare
getFilesetRoute’s public return type as string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c09b679f-93af-4abb-8e4d-9be459f17872
📒 Files selected for processing (5)
web/packages/studio/src/components/dataViews/AgentEvaluationsDataView/index.tsxweb/packages/studio/src/components/evaluation/Jobs/DetailsPanel.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/AgentEvaluationDetailRoute.tsxweb/packages/studio/src/routes/utils.test.tsweb/packages/studio/src/routes/utils.ts
|
https://nvbugspro.nvidia.com/bug/6581177
Summary by CodeRabbit
Bug Fixes
Tests