feat(experiments): Experiments UI misc QA#296
Open
shanaiabuggy wants to merge 7 commits into
Open
Conversation
Signed-off-by: shanaiabuggy <59746633+shanaiabuggy@users.noreply.github.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a server-side ChangesExperiment Groups Count and Navigation Rebranding
Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/packages/studio/src/routes/ExperimentGroupDetailRoute/index.tsx (1)
19-19: 🏗️ Heavy liftHandle loading and error states for
useGetExperimentGroup.
ExperimentRouteexplicitly handles loading/error states; this route should follow the same pattern for consistency.♻️ Suggested implementation pattern
Destructure
isLoadinganderror:- const { data: group } = useGetExperimentGroup(workspace, experimentGroupName); + const { data: group, isLoading, error } = useGetExperimentGroup(workspace, experimentGroupName);Add loading and error UI before the main render (similar to
ExperimentRoute/index.tsxlines 47-61):+ if (isLoading) { + return <Loading description="Loading experiment group..." />; + } + + if (error) { + return ( + <StatusMessage + className="mx-auto mt-density-2xl" + size="medium" + slotMedia={<CircleAlert width={65} height={65} />} + slotHeading="Error loading experiment group" + slotSubheading={error.message} + /> + ); + }🤖 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/ExperimentGroupDetailRoute/index.tsx` at line 19, The hook call in ExperimentGroupDetailRoute only reads data and misses loading/error handling; update the useGetExperimentGroup call to destructure isLoading and error (e.g., const { data: group, isLoading, error } = useGetExperimentGroup(workspace, experimentGroupName)), then add an early return before the main render that shows the same loading UI when isLoading is true and an error UI when error is present (matching the pattern used in ExperimentRoute) so the component consistently handles fetch states.web/packages/studio/src/routes/ExperimentRoute/ExperimentGroupCard.tsx (1)
26-26: ⚡ Quick winConsider handling loading/error states for experiment count.
When the experiments query is loading or fails, the count defaults to "0", which users can't distinguish from an actual zero count. Destructuring
isLoadinganderrorfrom the hook would allow you to show a skeleton, error indicator, or placeholder.💡 Example enhancement
- const { data: experimentsData } = useListExperiments(workspace, { + const { data: experimentsData, isLoading: isLoadingCount, error: countError } = useListExperiments(workspace, { filter: { experiment_group_id: group.id }, page_size: 1, }); - const experimentCount = experimentsData?.pagination?.total_results ?? 0; + const experimentCount = isLoadingCount + ? '...' + : countError + ? '—' + : String(experimentsData?.pagination?.total_results ?? 0);Then render as:
- <Metric title="Experiments" value={String(experimentCount)} /> + <Metric title="Experiments" value={experimentCount} />🤖 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/ExperimentRoute/ExperimentGroupCard.tsx` at line 26, The experiments count currently falls back to 0 via the experimentCount = experimentsData?.pagination?.total_results ?? 0 assignment, which hides loading/error states; update the hook call inside ExperimentGroupCard to destructure isLoading and error (e.g., const { data: experimentsData, isLoading, error } = useYourExperimentsHook(...)), then replace direct rendering of experimentCount with conditional rendering: show a skeleton/placeholder while isLoading, an error indicator when error is truthy, and only use experimentsData?.pagination?.total_results when neither loading nor error; keep the experimentCount variable but only compute it when not loading/error to avoid showing a misleading 0.
🤖 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/ExperimentGroupDetailRoute/index.tsx`:
- Line 19: The hook call in ExperimentGroupDetailRoute only reads data and
misses loading/error handling; update the useGetExperimentGroup call to
destructure isLoading and error (e.g., const { data: group, isLoading, error } =
useGetExperimentGroup(workspace, experimentGroupName)), then add an early return
before the main render that shows the same loading UI when isLoading is true and
an error UI when error is present (matching the pattern used in ExperimentRoute)
so the component consistently handles fetch states.
In `@web/packages/studio/src/routes/ExperimentRoute/ExperimentGroupCard.tsx`:
- Line 26: The experiments count currently falls back to 0 via the
experimentCount = experimentsData?.pagination?.total_results ?? 0 assignment,
which hides loading/error states; update the hook call inside
ExperimentGroupCard to destructure isLoading and error (e.g., const { data:
experimentsData, isLoading, error } = useYourExperimentsHook(...)), then replace
direct rendering of experimentCount with conditional rendering: show a
skeleton/placeholder while isLoading, an error indicator when error is truthy,
and only use experimentsData?.pagination?.total_results when neither loading nor
error; keep the experimentCount variable but only compute it when not
loading/error to avoid showing a misleading 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e9fbe7ce-a26e-47ed-8626-3366699b90bd
📒 Files selected for processing (8)
web/packages/studio/src/components/ExperimentGroupCreateModal/index.tsxweb/packages/studio/src/components/dataViews/ExperimentGroupDataView/index.tsxweb/packages/studio/src/components/dataViews/ExperimentSessionsDataView/index.tsxweb/packages/studio/src/routes/ExperimentDetailRoute/ExperimentDetailMetrics.tsxweb/packages/studio/src/routes/ExperimentDetailRoute/index.tsxweb/packages/studio/src/routes/ExperimentGroupDetailRoute/index.tsxweb/packages/studio/src/routes/ExperimentRoute/ExperimentGroupCard.tsxweb/packages/studio/src/routes/ExperimentRoute/index.tsx
Contributor
|
nakolean
reviewed
Jun 12, 2026
Signed-off-by: shanaiabuggy <59746633+shanaiabuggy@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@services/intake/src/nmp/intake/api/v2/experiments/endpoints.py`:
- Around line 185-189: The update path returns
ExperimentGroupResponse.from_entity(updated) without populating
experiment_count, causing inconsistent responses; update the
update_experiment_group handler to set response.experiment_count the same way as
the other endpoint by calling await
_count_live_experiments_in_group(entity_client, workspace=workspace,
group_id=updated.id) and assigning it to the response returned (i.e., after
creating response = ExperimentGroupResponse.from_entity(updated) set
response.experiment_count = ...), ensuring you use the same workspace and
entity_client variables as the other endpoint.
- Around line 150-155: The code is doing an unbounded fan-out by calling
_count_live_experiments_in_group once per group (the asyncio.gather over
_count_live_experiments_in_group for each g in result.data), which creates an
N+1 hotspot; replace the per-group parallel calls with a single batched/grouped
count call that accepts all group IDs and returns counts per group (e.g.
implement count_live_experiments_by_group(entity_client, workspace, group_ids)
that issues one query with GROUP BY or an IN filter and maps results to group
IDs), then use that mapping to populate counts for each g in result.data instead
of awaiting many separate _count_live_experiments_in_group calls. Ensure callers
of counts use the new mapping and that page_size/large result pages are handled
efficiently.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 07df0815-9fdd-4fed-a838-4f2a86df2622
📒 Files selected for processing (10)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlservices/intake/src/nmp/intake/api/v2/experiments/endpoints.pyservices/intake/src/nmp/intake/api/v2/experiments/schemas.pyweb/packages/studio/src/components/dataViews/ExperimentGroupDataView/index.tsxweb/packages/studio/src/routes/ExperimentDetailRoute/ExperimentDetailMetrics.tsxweb/packages/studio/src/routes/ExperimentDetailRoute/index.tsxweb/packages/studio/src/routes/ExperimentGroupDetailRoute/index.tsxweb/packages/studio/src/routes/ExperimentRoute/ExperimentGroupCard.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- web/packages/studio/src/routes/ExperimentGroupDetailRoute/index.tsx
- web/packages/studio/src/routes/ExperimentDetailRoute/ExperimentDetailMetrics.tsx
Signed-off-by: shanaiabuggy <59746633+shanaiabuggy@users.noreply.github.com>
Signed-off-by: shanaiabuggy <59746633+shanaiabuggy@users.noreply.github.com>
Signed-off-by: shanaiabuggy <59746633+shanaiabuggy@users.noreply.github.com>
Signed-off-by: shanaiabuggy <59746633+shanaiabuggy@users.noreply.github.com>
nakolean
approved these changes
Jun 12, 2026
BrianNewsom
approved these changes
Jun 12, 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
3 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.
ExperimentstoExperiment Groupsacross the index, group detail, and experiment detail routes.Create group.Model Namesrenamed toModelsExperimentssection header with a count badge above the experiments list, separated by a top border.Agent NamewithDataset Name+Dataset Version, addedModels(with tooltip-on-truncate for multi-model cases), and right-justified the rollup-derived KVs (Models,Avg Cost,Avg Latency).Test casessection header with a count badge above the sessions list on the experiment detail page, separated by a top border (matches the Experiments header pattern).Summarylabel above the summary text, instead of a bare string.Screen.Recording.2026-06-12.at.1.09.03.AM.mov
Summary by CodeRabbit
New Features
UI Improvements