fix(studio): experiment detail view UI polish (ASE-365/366/367) - #466
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughExperiment sessions update input rendering and numeric alignment, experiment detail metrics conditionally show dataset version in the Dataset Name tooltip, and experiment routes add a lazy-loaded trace detail route. ChangesExperiment views and trace route
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 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
`@web/packages/studio/src/routes/ExperimentDetailRoute/ExperimentDetailMetrics.tsx`:
- Around line 46-60: The dataset name is being wrapped in a Tooltip even when
there is no dataset_version to display, which makes the UI suggest hover content
that does not exist. Update ExperimentDetailMetrics so the Tooltip is only
rendered when experiment.dataset_version is present, and keep the plain Text
rendering path for the no-version case; use the existing
experiment.dataset_name, experiment.dataset_version, and Tooltip/Text branches
to locate 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ab9d542c-0022-4cef-9fea-af13625e70ac
📒 Files selected for processing (3)
packages/nmp_common/src/nmp/common/observability/tracing.pyweb/packages/studio/src/components/dataViews/ExperimentSessionsDataView/index.tsxweb/packages/studio/src/routes/ExperimentDetailRoute/ExperimentDetailMetrics.tsx
Contributor
|
Contributor
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
`@web/packages/studio/src/components/dataViews/ExperimentSessionsDataView/index.tsx`:
- Around line 142-145: The full-input trigger in ExperimentSessionsDataView is
mouse-only because the clickable Text relies on onClick when isLong is true.
Update the Text interaction so it is keyboard accessible by either rendering a
proper button/link-style control or by adding focusability, role, and
Enter/Space key handling in the same ExperimentSessionsDataView component. Keep
the existing setInputModalValue and stopPropagation behavior, but ensure the
long-value trigger can be reached and activated without a mouse.
🪄 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: b002609f-2527-48d8-bf18-15bc34d08954
📒 Files selected for processing (2)
web/packages/studio/src/components/dataViews/ExperimentSessionsDataView/index.tsxweb/packages/studio/src/routes/ExperimentDetailRoute/ExperimentDetailMetrics.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web/packages/studio/src/routes/ExperimentDetailRoute/ExperimentDetailMetrics.tsx
Contributor
|
That's a lot of popups to try to show the full input. Let's remove the preview and the full view and rely on clicking through the row to see the trace. |
walston
force-pushed
the
nwalston/ui-bugs
branch
from
June 25, 2026 19:59
8eef16a to
634d45a
Compare
Replaces single-line truncation (truncate + max-w) with line-clamp-2 on the Input cell in ExperimentSessionsDataView, and widens the column from 240px to 400px so more of the value is visible before clamping. Also fixes a related local-dev CORS issue (ASE-365 blocker): skip FastAPIInstrumentor.instrument_app when no trace exporter is configured. The OTel ASGI wrapper sits outside CORSMiddleware and throws on OPTIONS preflight route resolution (see ASTD-267), causing 500s on all API preflight requests in local dev. Signed-off-by: Nathan Walston <nwalston@nvidia.com>
…(ASE-366) Remove the standalone Dataset Version KV pair from the experiment detail header. The version is now shown in a Tooltip on the Dataset Name value, visible on hover. Keeps the header compact when dataset versions are long (e.g. sha256 hashes). Signed-off-by: Nathan Walston <nwalston@nvidia.com>
… (ASE-367)
Add meta: { alignment: 'right' } to Latency, Tokens, Cost, and evaluator
score columns. The alignment flows through to both the header cell and body
cells via the DataView's existing meta.alignment support.
Signed-off-by: Nathan Walston <nwalston@nvidia.com>
… follow-up) Long input values caused the tooltip to exceed viewport height, triggering CSS Anchor Positioning fallback to margin:auto centering — which placed the tooltip on the trigger and immediately closed it. Fix: truncate tooltip content at 400 chars. For values over 400 chars, append a styled hint and wire the cell click to open a Modal with the full text (click stops row-navigation propagation). Tooltip truncation guarantees anchor positioning always finds a valid above/below slot. Signed-off-by: Nathan Walston <nwalston@nvidia.com>
…false }
Per nkolean: use per-column meta: { title: false } instead of the table-wide
autoCellTooltips=false approach, so other columns keep their native title
tooltips. The input column manages its own Tooltip.
Signed-off-by: Nathan Walston <nwalston@nvidia.com>
…SE-366) Per CodeRabbit: the Tooltip trigger was unconditionally wrapping the dataset name even when dataset_version is absent, advertising hover content that doesn't exist. Render plain text when no version is available. Signed-off-by: Nathan Walston <nwalston@nvidia.com>
…h (ASE-365) Per rrhyne: tooltip previews and the full-value modal add unnecessary complexity. Experiments always have trace_ids so the existing row click-through to trace detail is sufficient. Input cell is now line-clamp-2 only. Signed-off-by: Nathan Walston <nwalston@nvidia.com>
Signed-off-by: Nathan Walston <nwalston@nvidia.com>
rrhyne
approved these changes
Jun 26, 2026
rrhyne
left a comment
rrhyne
left a comment
Contributor
There was a problem hiding this comment.
Looks great, thanks for making that change.
walston
enabled auto-merge
June 26, 2026 15:01
walston
disabled auto-merge
June 26, 2026 15:01
walston
force-pushed
the
nwalston/ui-bugs
branch
from
June 26, 2026 15:02
649767c to
95a1f66
Compare
walston
enabled auto-merge
June 26, 2026 18:50
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.
Closes #ASE-365, #ASE-366, #ASE-367
Screen.Recording.2026-06-25.at.15.11.31.mov
Summary
Three cosmetic fixes to the experiment detail view, all found while reviewing real switchyard data in the dev instance.
ASE-365 — Input column truncation. Long prompt strings were blowing out row height because
truncatedepends onwhite-space: nowrap, which theTextcomponent overrides. Replaced withline-clamp-2(webkit line clamp, layout-independent) and widened the column from 240 px to 400 px so more of the value is readable before clamping. Full text remains accessible on hover via the existing tooltip.ASE-366 — Dataset version popover. The dataset version field (often a long sha256 hash) occupied a full KV slot in the header, pushing other metadata off-screen. Removed the standalone slot and moved the version into a hover tooltip on the Dataset Name value.
ASE-367 — Right-align numeric columns. Latency, Tokens, Cost, and evaluator score columns now carry
meta: { alignment: 'right' }, which the DataView's existingTableHeaderCell/body cellalignprop picks up. Text and date columns are unchanged.Also fixes a local-dev CORS regression (OPTIONS preflight 500s) caused by
FastAPIInstrumentor.instrument_appwrapping the entire ASGI stack outsideCORSMiddleware. Skipping the wrapper when no trace exporter is configured unblocks local development without affecting production deployments (see ASTD-267 for the upstream fix).Test plan
…and hovering shows the full valueSummary by CodeRabbit
New Features
Enhancements