[OPIK-7041] [FE] feat: Optimization runs list — columns, Item source, Discarded + refactors#7227
Conversation
…tor cleanup Align the v2 Optimization runs list to the Figma (562:37189): - Name is the first column; add Run ID / Algorithm / Metric and reuse the Experiments "Item source" cell (renamed from "Dataset name", with the missing icon tooltip). New columns ship available-but-hidden, matching the Figma default-visible set (Name, Start time, Status, Pass rate, Accuracy, Latency, Cost, Opt. cost). - Rename trial status "Pruned" -> "Discarded" (v2 label only; key unchanged). Fold-in refactors (v2 only): - Extract OptimizationsColumns + OptimizationsToolbar; slim OptimizationsPage. - Drop the ghost `deploy` column and bump column-prefs keys v1 -> v2. - Gate the 30s list polling on in-progress runs via an additive, v1-safe param on the shared useOptimizationsView hook. - Remove the orphaned v2 DatasetNameCell. Implements OPIK-7041 (part of OPIK_6528). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| cell: ItemSourceCell as never, | ||
| customMeta: { | ||
| nameKey: "dataset_name", | ||
| idKey: "dataset_id", | ||
| resource: RESOURCE_TYPE.testSuite, | ||
| }, |
There was a problem hiding this comment.
Idless optimization rows show blank source
OptimizationPublic.dataset_id is optional, so optimizations can arrive with dataset_name but no id, and routing this column through ItemSourceCell with idKey: "dataset_id" makes those rows fall back to "-" instead of showing the source label — should we keep rendering dataset_name and only hide the link/icon when the id is missing?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/v2/pages/OptimizationsPage/OptimizationsColumns.tsx around lines
50-60, the `dataset_name` column routes rendering through `ItemSourceCell` with `idKey:
"dataset_id"`, which causes the entire source label to disappear (becoming `"-"`) when
`dataset_id` is missing. Refactor the `dataset_name` column so it always displays
`row.dataset_name` (using `accessorFn` or a wrapper cell) and only hides/disables the
link/icon portion when `row.dataset_id` is falsy. If needed, introduce a small wrapper
renderer in this file that passes `idKey`/resource for link behavior but falls back to
`dataset_name` for the visible label regardless of id presence.
There was a problem hiding this comment.
Good catch — valid regression, fixed in 5cb26c8. ItemSourceCell now falls back to rendering the source name (without a link) when there's no linkable id, instead of "-", so idless optimization rows keep their label. Applied in the shared cell so Experiments benefits too (idless rows there previously also showed a bare dash); typecheck + the experiments/optimization test suites stay green.
There was a problem hiding this comment.
Commit 5cb26c8 addressed this comment by changing ItemSourceCell to fall back to rendering the source name when no linkable id is present. That keeps idless optimization rows showing dataset_name while omitting only the link/icon behavior.
ItemSourceCell rendered "-" when the row had no linkable id, so an optimization row with a dataset_name but no dataset_id (the REST type makes it optional) lost its source label. Fall back to the name (no link) instead of a bare dash. Addresses PR review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stence probe The runs list derived `isEmpty` from the paginated fetch gated on `!isTableLoading`, so a cold load with no data flashed the studio templates + a populated-looking table skeleton before snapping to the empty state. Decide emptiness from a lightweight unfiltered size=1 query instead (mirrors ExperimentsPage), fold that probe into `isTableLoading` so the table shows its skeleton while it resolves, and keep the studio templates out of the loading state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- empty state: treat only a *successful* size=1 probe with total 0 as empty, so a transient probe failure no longer hides real runs behind onboarding - polling: detect RUNNING runs workspace-wide via an active-runs probe so paginating away from page 1 no longer silently stops the auto-refresh (cross-page INITIALIZED / cross-tab starts documented as edge cases) - Algorithm / Metric cells render "-" instead of a blank cell when absent - search placeholder now matches the relabeled "Item source" column - dedupe the local in-progress statuses to the shared IN_PROGRESS_OPTIMIZATION_STATUSES constant Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| const { data: existenceData, isPending: isExistencePending } = | ||
| useOptimizationsList({ | ||
| workspaceName, | ||
| projectId: activeProjectId ?? undefined, | ||
| page: 1, |
There was a problem hiding this comment.
Duplicate empty-state probe
useOptimizationsList adds the same page=1/size=1 existence probe and isExistencePending gating that already exists in ExperimentsPage.tsx and GeneralDatasetsTab.tsx, so any tweak to the empty-state or refetch logic has to be copied to three places — should we extract a shared useListExistenceProbe hook?
Want Baz to fix this for you? Activate Fixer
There was a problem hiding this comment.
Agreed it's worth a shared useListExistenceProbe. Holding it out of this PR though: the probe also lives in ExperimentsPage and GeneralDatasetsTab, so extracting it touches the Experiments and Datasets features unrelated to this optimization-runs change. Tracking as a focused follow-up so that refactor (and its regression surface) is reviewed on its own.
There was a problem hiding this comment.
That makes sense — keeping the cross-feature refactor in a focused follow-up PR is a reasonable boundary here. I'll save this to memory once the PR is merged.
| workspaceName, | ||
| projectId, | ||
| datasetId: datasetId || "", | ||
| filters: ACTIVE_OPTIMIZATION_FILTER, |
There was a problem hiding this comment.
Cross-page INITIALIZED stops polling
The workspace-wide ACTIVE_OPTIMIZATION_FILTER only queries status=running, so INITIALIZED optimizations on other pages keep activeData.total at 0 and auto-refresh stops while work is still active elsewhere — should we include INITIALIZED in that probe?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/hooks/useOptimizationsView.ts around lines 47-63 inside the
`useOptimizationsView` hook, the workspace-wide “active runs” probe calls
`useOptimizationsList` with `filters: ACTIVE_OPTIMIZATION_FILTER`, which only covers
`status=running`, so `hasActiveRuns` won’t be true when the only active optimization
elsewhere is `INITIALIZED`. Refactor by updating the probe filters to include
`INITIALIZED` as an active/in-progress state (either modify `ACTIVE_OPTIMIZATION_FILTER`
to include both statuses, or introduce a dedicated workspace-active filter constant for
`RUNNING` + `INITIALIZED`), and then compute `hasActiveRuns` from that expanded query.
This ensures the `refetchInterval` continues polling while work is active on other pages
for v2 callers.
There was a problem hiding this comment.
Intentional, documented tradeoff (see the comment block on the probe). The workspace-wide probe detects RUNNING; a cross-page INITIALIZED run is transient (flips to RUNNING almost immediately) and is still caught by the per-page IN_PROGRESS_OPTIMIZATION_STATUSES check once it surfaces, plus the next window-focus refetch. Widening the probe cleanly needs a multi-status (in) status filter — happy to add RUNNING+INITIALIZED if this proves noticeable, but keeping the single-status probe for now rather than ship an unverified status in query.
There was a problem hiding this comment.
Thanks, that makes sense — I’ll keep this tradeoff in mind and save it to memory once the PR is merged.
… Figma Set the default-visible column widths to the runs-list table header in Figma (686:35271 inside 562:37189): Name 191, Start time 145, and Status / Pass rate / Accuracy / Latency / Cost / Opt. cost all 120 (previously 220 / 140 / 200 / 200 / 180 / 180). Hidden columns (Run ID, Item source, Algorithm, Metric) keep their defaults — they aren't in the Figma default-visible frame. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| id: "pass_rate", | ||
| label: "Pass rate", | ||
| type: COLUMN_TYPE.numberDictionary, | ||
| size: 120, | ||
| accessorFn: (row) => row.best_objective_score, | ||
| cell: OptimizationPassRateCell as never, | ||
| }, | ||
| { | ||
| id: "accuracy", | ||
| label: "Accuracy", | ||
| type: COLUMN_TYPE.numberDictionary, | ||
| size: 120, |
There was a problem hiding this comment.
pass_rate, accuracy, latency, cost, and opt_cost column defs are duplicated across v1 and v2 with size: 120 hard-coded four times, so any metric tweak or width change (per the PR, 120 is the Figma-matched width) has to be kept in sync everywhere — should we extract a getOptimizationMetricColumns({ width }) helper and a DEFAULT_METRIC_COLUMN_WIDTH = 120 constant and reuse them?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Extract a shared `getOptimizationMetricColumns({ width })` helper reused by both v1 and
v2 column definitions. Introduce a named constant `DEFAULT_METRIC_COLUMN_WIDTH = 120`
near the existing width constants/comments, and replace all hard-coded `size: 120`
literals in the metric columns (lines 101, 109, 118, 126) with this constant. This makes
the Figma-spec intent explicit and prevents drift when adding/removing metric columns or
syncing across files.
There was a problem hiding this comment.
Commit b474cef addressed this comment by introducing DEFAULT_METRIC_COLUMN_WIDTH = 120 and replacing the repeated size: 120 literals in the v2 metric columns with that constant. This removes the hard-coded width duplication in this file and makes the intended metric width explicit.
…list (Figma 1:1) The with-data runs list showed only 2 onboarding cards (a demo template + a hardcoded "Optimize your own prompt"); the Figma frame (562:37189) specifies three: Run a demo example, Use the Optimization studio, and Optimize via SDK. Rebuild StudioTemplates as the 3-card layout matching Figma — coloured icon chip + title + description + action link per card — and wire the SDK card to the same dialog the empty state uses. Removes the now-unused OptimizationTemplateCard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| // Three onboarding cards, matching the Figma runs-list (562:37189). | ||
| const cards: StudioCard[] = [ | ||
| { | ||
| icon: BotMessageSquare, | ||
| color: "var(--chart-blue)", |
There was a problem hiding this comment.
Duplicate studio action cards
The cards array duplicates the actionCards list in OptimizationsEmptyState, so text, color, or callback changes have to be updated in two places and the views can drift apart — should we extract the shared card config or renderer into a common module?
Want Baz to fix this for you? Activate Fixer
There was a problem hiding this comment.
Commit b474cef addressed this comment by extracting the shared onboarding card definitions into getStudioCardConfigs and reusing them in both StudioTemplates and OptimizationsEmptyState. This removes the duplicated title/description/icon/callback setup so the two views can’t drift apart as easily.
…ds and the list Figma 562:37189 has a full-width horizontal separator (686:35245) between the "Run an optimization" cards and the search/table; the implementation was missing it. Render a Separator after the cards (gated with them, so it only shows alongside the onboarding section). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cells to Figma
- Metric column now maps objective_name through OPTIMIZATION_METRIC_OPTIONS
(new getMetricLabel) so it shows human copy ("Levenshtein") instead of the raw
metric key, mirroring how Algorithm uses getOptimizerLabel.
- Metric comparison cell (compact, used by the runs list) aligned 1:1 to Figma
562:37189: 12px baseline (muted) and current (foreground, regular weight,
previously 14px/bold), and the up/good trend icon uses the vivid Figma green
(#00D14C) via a new PercentageTrend `brightPositive` flag — scoped to the
green direction so negative/neutral trends keep their colors, and only the
compact (optimization) cell opts in, leaving KPI cards untouched.
Note: the Algorithm column was empty only for runs without studio_config; it
already resolves studio_config.optimizer.type via getOptimizerLabel for real
runs (no code change needed).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| export const getMetricLabel = (objectiveName?: string): string => { | ||
| if (!objectiveName) return "-"; | ||
| return ( | ||
| OPTIMIZATION_METRIC_OPTIONS.find((opt) => opt.value === objectiveName) | ||
| ?.label || objectiveName | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Metric label helper duplication
getMetricLabel duplicates the OPTIMIZATION_METRIC_OPTIONS lookup and fallback already used in lib/optimization-config.ts and the v1 OptimizationConfiguration helper, so metric naming changes have to be synced in three places and can drift — should we reuse the shared helper here instead of copying it again?
Want Baz to fix this for you? Activate Fixer
There was a problem hiding this comment.
Commit b474cef addressed this comment by removing the duplicated getMetricLabel helper from apps/opik-frontend/src/lib/optimizations.ts and switching the table code to import getMetricLabel from the shared lib/optimization-config helper instead. That centralizes the metric-name lookup/fallback instead of keeping a third copy here.
| const iconColorClassName = | ||
| brightPositive && variant === "green" ? "text-[#00d14c]" : undefined; |
There was a problem hiding this comment.
Bright-positive icon bypasses theme tokens
brightPositive maps the icon to text-[#00d14c], and MetricComparisonCell enables that path, so the vivid green is hard-coded in the UI instead of coming from theme tokens — should we add a CSS variable for it in src/main.scss under :root and .dark, then switch to text-[var(--...)] as .agents/skills/opik-frontend/ui-components.md suggests?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/shared/PercentageTrend/PercentageTrend.tsx around lines 59-61
(the `iconColorClassName` logic inside `PercentageTrend`), remove the hard-coded
Tailwind class `text-[#00d14c]` for `brightPositive` and replace it with a theme-aware
class that references a CSS variable (e.g., `text-[var(--...)]`). Then update
src/main.scss (near the existing :root and .dark color/token definitions) to add a new
token for this vivid positive green shade so it works correctly in dark mode. Ensure
`brightPositive` still only applies when `variant === "green"`, but the color source is
the new CSS variable rather than the literal hex.
There was a problem hiding this comment.
Commit 9f323b3 addressed this comment by adding a shared --color-green-bright CSS variable in src/main.scss and switching the bright-positive icon class to text-[var(--color-green-bright)] via TREND_COLOR_CLASS. The related MetricComparisonCell change now reuses the shared trend color mapping instead of relying on a hard-coded green hex.
…cell to Figma
Cards (686:35206): rebuilt to the Figma layout — horizontal, a solid colored
icon chip (sky/violet/fuchsia, white icon) + title/description + a primary-blue
link ("Try template" / "Create optimization" / "View SDK guide"), on a tinted
card with a small shadow. Previously a hand-rolled vertical card with tinted
chips and per-color links.
Status (686:35286): the run status is now a neutral pill with a colored status
dot, not a colored-background badge — running = green (#00d14c), completed /
initialized = slate, cancelled / error = red (#ef4444), per Figma. Extracted as
a shared OptimizationStatusTag (no existing shared cell fit run statuses:
StatusCell is boolean, PassedCell's StatusTag is test-suite assertions) so the
list, run-detail header and trials can render it identically.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| chipColor: "#89deff", | ||
| tintBg: "rgba(186, 230, 253, 0.1)", | ||
| tintBorder: "rgba(186, 230, 253, 0.6)", |
There was a problem hiding this comment.
Theme tokens bypassed for cards
chipColor, tintBg, and tintBorder are hard-coded here, so the onboarding cards bypass the CSS-variable theme path and dark mode keeps the light tints — should we move them to main.scss vars/classes and add .dark overrides, as .agents/skills/opik-frontend/ui-components.md suggests?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/v2/pages-shared/optimizations/StudioTemplates/StudioTemplates.tsx
around lines 38-62, the `cards` array hard-codes `chipColor`, `tintBg`, and `tintBorder`
as hex/rgba, which breaks theme awareness (especially dark mode) and violates
ui-components.md’s requirement to use CSS variables defined in `main.scss` under
`:root` and `.dark`. Refactor by introducing new CSS variables for these three palettes
(mirroring the existing upload-chip variables pattern like `--upload-chip-bg`,
`--upload-chip-border`, `--upload-chip-icon-bg`), add dark-mode `.dark` overrides if
needed, and update the card rendering to use theme-aware classNames/variables instead of
inline color literals. Ensure the `style={{ backgroundColor: tintBg, borderColor:
tintBorder }}` and chip `style` are replaced/derived from the new tokens so
DatasetCsvDropzone-like behavior is consistent across light/dark.
There was a problem hiding this comment.
Partly addressed: b474cef extracts getStudioCardConfigs so the card copy/icon/routing is shared between the populated cards and the empty state. The remaining bit (chip/tint colors -> main.scss vars + .dark) I'd rather not guess at — these are Figma light-mode-specific tints with no dark equivalent yet. Flagging for design to provide the dark-mode card treatment, then I'll tokenize in a follow-up.
There was a problem hiding this comment.
Thanks, that makes sense — noted. I'll save this to memory once the PR is merged.
| const STATUS_DOT_COLOR: Record<OPTIMIZATION_STATUS, string> = { | ||
| [OPTIMIZATION_STATUS.RUNNING]: "#00d14c", | ||
| [OPTIMIZATION_STATUS.COMPLETED]: "#94a3b8", | ||
| [OPTIMIZATION_STATUS.CANCELLED]: "#ef4444", | ||
| [OPTIMIZATION_STATUS.INITIALIZED]: "#94a3b8", | ||
| [OPTIMIZATION_STATUS.ERROR]: "#ef4444", |
There was a problem hiding this comment.
Hard-coded status colors ignore dark theme
STATUS_DOT_COLOR and the <Tag> styles use raw hex values instead of the theme variables from main.scss, so the status pill stays on light-mode colors in dark mode — should we move these to CSS variables or theme-aware utility classes?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/v2/pages-shared/optimizations/OptimizationStatusTag/OptimizationStatusTag.tsx
around lines 10-15 and 27-37, the `STATUS_DOT_COLOR` map and the `Tag` className
(`border-[#f1f5f9] bg-[#f8fafc]`) hard-code light-mode hex colors instead of using the
existing theme CSS variables/tokens used by `main.scss` and its dark overrides. Refactor
by replacing the hex values with theme-aware CSS variables (e.g., map each
`OPTIMIZATION_STATUS` to a `var(--...)` token) and update the `Tag` styling to use theme
tokens or utility classes that resolve to dark-mode values. If the needed tokens don’t
exist yet, add them to the theme stylesheet (main.scss and dark overrides) and reference
those variables from this component so the status tag tracks both themes.
There was a problem hiding this comment.
Commit 9f323b3 addressed this comment by replacing the hard-coded status dot hex values with theme CSS variables (var(--color-green-bright), var(--color-red), hsl(var(--light-slate))). That makes the status indicator theme-aware instead of fixed to light-mode colors.
…ric cells The Pass rate / Latency / Cost comparison cells now color the trend icon with the vivid Figma trend palette — good = green (#00D14C), bad = red (#EF4444) — via a `vivid` flag on PercentageTrend (renamed from brightPositive, now covers both directions; neutral unchanged). Scoped to the compact metric cell, so KPI cards and other PercentageTrend usages are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the per-cell `vivid` flag on PercentageTrend (smelly: it made the optimization metric cells diverge from every other trend in the product). PercentageTrend now always uses the Figma trend palette — good = green (#00D14C), bad = red (#EF4444), neutral = gray — applied to the whole tag, so every trend indicator (optimization cells, KPI cards, traces metrics, etc.) renders identically. MetricComparisonCell no longer passes a flag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… tokens Same smell as the trend flag: the run-status dots hardcoded #00d14c / #ef4444 — the exact green/red PercentageTrend also hardcoded, so the two could drift. Add a single --color-green-bright brand token (#00d14c) and route both through it, reuse the existing --color-red and --light-slate tokens. No magic hexes left in either component; one source of truth keeps trend and status colors in sync. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| // slate, cancelled/error = red. Uses the shared brand-color tokens (same green/ | ||
| // red as PercentageTrend) so trend and status colors stay in sync. Shared so | ||
| // the list, run-detail header and trials render the status identically. | ||
| const STATUS_DOT_COLOR: Record<OPTIMIZATION_STATUS, string> = { | ||
| [OPTIMIZATION_STATUS.RUNNING]: "var(--color-green-bright)", | ||
| [OPTIMIZATION_STATUS.COMPLETED]: "hsl(var(--light-slate))", | ||
| [OPTIMIZATION_STATUS.CANCELLED]: "var(--color-red)", | ||
| [OPTIMIZATION_STATUS.INITIALIZED]: "hsl(var(--light-slate))", | ||
| [OPTIMIZATION_STATUS.ERROR]: "var(--color-red)", |
There was a problem hiding this comment.
Inconsistent optimization status colors
OptimizationStatusTag centralizes the status color mapping, but the v2 detail header still uses its own STATUS_TO_VARIANT_MAP, so the list and header can show different colors for the same status — should we reuse the shared map there too?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/v2/pages-shared/optimizations/OptimizationStatusTag/OptimizationStatusTag.tsx
around lines 8-16 (the status-to-color mapping), ensure this shared
OPTIMIZATION_STATUS→color map is the single source of truth. Refactor any other
components (notably the v2 detail header and trials/list renderers) that still define
their own STATUS_TO_VARIANT_MAP to instead import and use the same mapping (or route
through OptimizationStatusTag) so RUNNING/COMPLETED/INITIALIZED/CANCELLED/ERROR render
identical colors across surfaces. Verify by checking the header and list/trials for each
status and removing/unused duplicate maps after wiring them to the shared one.
There was a problem hiding this comment.
Fixed in b474cef. The v2 run-detail header now renders the shared OptimizationStatusTag instead of its own STATUS_TO_VARIANT_MAP, so the list and header show identical status colors + the same pill+dot styling.
There was a problem hiding this comment.
Commit b474cef addressed this comment by replacing the v2 detail header's local STATUS_TO_VARIANT_MAP/Tag usage with the shared OptimizationStatusTag. That makes the header use the same status-to-color mapping as the list, eliminating the inconsistent colors for the same optimization status.
| const TREND_COLOR_CLASS: Record<string, string | undefined> = { | ||
| green: "text-[var(--color-green-bright)]", | ||
| red: "text-[var(--color-red)]", |
There was a problem hiding this comment.
Low-contrast trend tags
PercentageTrend now uses text-[var(--color-green-bright)] / text-[var(--color-red)] on the existing Tag pill backgrounds, so the percentage label and lucide icon lose contrast and become hard to read in every v1/v2 caller — should we keep --tag-green-text / --tag-red-text here, or switch to darker trend tokens?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/shared/PercentageTrend/PercentageTrend.tsx around lines 48-50
(the trend style color mapping for `green`/`red`), the component uses
`text-[var(--color-green-bright)]` / `text-[var(--color-red)]`, but the Tag backgrounds
remain `bg-[var(--tag-green-bg)]` / `bg-[var(--tag-red-bg)]`. Refactor the color mapping
so the `green`/`red` variants use the Tag’s paired text token(s) (e.g.,
`--tag-green-text` / `--tag-red-text`) or switch to a darker trend token that preserves
contrast on those pill backgrounds, ensuring both the percentage label and lucide icon
inherit the corrected text color. Verify readability/contrast for all v1/v2 callers
using the Tag background presets.
There was a problem hiding this comment.
Commit b474cef addressed this comment by removing the explicit bright trend text classes from PercentageTrend and letting the Tag variant supply its own paired text color. That restores contrast for both the percentage label and the lucide icon on the tinted tag backgrounds.
There was a problem hiding this comment.
Fixed in b474cef. Removed the bright-color override on PercentageTrend's Tag — it now uses the Tag variant's own text token, so the % label + icon keep proper contrast on the tinted tag background in every v1/v2 caller. The vivid green (--color-green-bright) now lives only in the runs-list metric pill, where the icon sits on a neutral #f8fafc surface (matches Figma 689:34516).
…nd % in neutral pill Redesign the runs-list Pass rate / Latency / Cost cells to Figma (689:34516, 496:41663, 496:41887): the change % now sits in a neutral pill (bg #f8fafc / border #f1f5f9 — the same surface as the status tag) with a colored trend icon and muted-slate text, followed by the current value in foreground. Drops the old baseline-value display in compact mode. Export getTrendConfig + TREND_COLOR_CLASS from PercentageTrend so the cell reuses the single trend->icon/color mapping instead of re-deriving it. Only the compact branch changes (runs list); PercentageTrend's other consumers and MetricComparisonCell's full/KPICard mode are untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| const trendConfig = | ||
| !isUndefined(percentage) && isFinite(percentage) | ||
| ? getTrendConfig(percentage, trend, 0) | ||
| : null; |
There was a problem hiding this comment.
The isFinite(percentage) guard blocks getTrendConfig() when calcFormatterAwarePercentage() returns Infinity (i.e. baseline === 0), so compact mode drops the trend icon entirely for zero-baseline comparisons — should we still compute the trend variant for non-finite values and just null out the percentageLabel?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
In
apps/opik-frontend/src/v2/pages-shared/experiments/MetricComparisonCell/MetricComparisonCell.tsx
around lines 35-38 inside the `compact` branch, refactor the `trendConfig` construction
so `getTrendConfig()` is still called for non-finite percentages (Infinity/-Infinity
from `baseline === 0`), preserving the icon-only `PercentageTrend` badge behavior. Keep
`percentageLabel` as null when the percentage is non-finite so no "%" text is shown, but
ensure `trendConfig.Icon` is always available for zero-baseline comparisons. Add or
update a unit test/story for `baseline === 0` cases to prevent regressions.
There was a problem hiding this comment.
Fixed in ac6687c. Compact mode now computes the trend variant whenever a percentage exists (including Infinity from a zero baseline) and only drops the numeric label when the value isn't finite — so zero-baseline comparisons keep their direction icon.
There was a problem hiding this comment.
Commit ac6687c addressed this comment by removing the isFinite(percentage) check from trendConfig creation, so compact mode still computes the trend icon for Infinity/-Infinity values from zero baselines. It also keeps percentageLabel null for non-finite percentages, preserving the icon-only badge behavior.
| <div className="flex items-center gap-1.5"> | ||
| {!isUndefined(baseline) && ( | ||
| <TooltipWrapper content={String(baseline)}> | ||
| <span className="comet-body-s text-muted-slate"> | ||
| {formatter(baseline)} | ||
| {trendConfig && TrendIcon && ( | ||
| <div className="inline-flex items-center gap-1 rounded-md border border-[#f1f5f9] bg-[#f8fafc] px-1.5 py-0.5"> |
There was a problem hiding this comment.
The compact pill uses hard-coded spacing (gap-1.5, px-1.5, py-0.5) and color classes (border-[#f1f5f9], bg-[#f8fafc]), so it drifts from the design system and won't follow theme switches — should we swap these for approved spacing steps and theme-aware tokens, as .agents/skills/opik-frontend/ui-components.md suggests?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/v2/pages-shared/experiments/MetricComparisonCell/MetricComparisonCell.tsx
around lines 46-48 (the `if (compact)` branch rendering the neutral trend pill div): 1.
Spacing: Replace non-standard utility classes `gap-1.5`, `px-1.5`, and `py-0.5` with
approved spacing steps from `.agents/skills/opik-frontend/ui-components.md` (e.g.,
closest allowed `gap-2|4|6|8` and `p-2|4|6` equivalents). If the design specifically
requires these exact values, add an explicit documented exemption reference in a
comment. 2. Colors: Remove hard-coded Tailwind color classes `border-[#f1f5f9]` and
`bg-[#f8fafc]`. Refactor to use existing theme-aware classes/tokens (e.g., the same
neutral surface styling used by the status tag) or, if no matching token exists, add a
dedicated CSS variable pair for the pill surface in `main.css` under `:root` and
`.dark`, then reference those variables via the component's className. Ensure the pill
background and border automatically switch in dark mode.
There was a problem hiding this comment.
The spacing matches the Figma cell spec (689:34516) and uses standard Tailwind steps (outer gap-1.5=6px, pill gap-1=4px, px-1.5=6px, py-0.5=2px). The neutral surface (#f8fafc/#f1f5f9) is deliberately identical to the shared OptimizationStatusTag pill — I'd rather tokenize that surface (with .dark overrides) across the status tag + metric pill together, as the same follow-up as the card-tokens comment, than diverge one pill now.
There was a problem hiding this comment.
Thanks, that makes sense — the spacing is intentional per Figma, and the shared neutral pill surface should be tokenized together rather than changed in one place first. I'll save this to memory once the PR is merged.
There was a problem hiding this comment.
Commit 4326ecd addressed this comment by replacing the pill’s hard-coded border/background colors with shared CSS variables (--pill-neutral-bg / --pill-neutral-border) and adding dark-mode values in main.scss. The compact pill now follows theme switches, even though the spacing classes were left as-is per the discussion.
- PercentageTrend: stop overriding the Tag text with the bright trend color so the percentage + icon keep proper contrast on the tinted tag background (baz: low-contrast trend tags). The bright green now lives only in the runs-list metric pill, where the icon sits on a neutral surface. - Run-detail header reuses the shared OptimizationStatusTag instead of its own STATUS_TO_VARIANT_MAP, so list and header render identical status colors. - Drop the duplicate getMetricLabel in lib/optimizations.ts; reuse the shared helper from lib/optimization-config.ts. - Extract DEFAULT_METRIC_COLUMN_WIDTH for the metric columns (was 120 hardcoded on each column def). - Extract getStudioCardConfigs so the populated runs-list cards and the empty state share one card definition (copy/icon/routing); each view keeps its own visual treatment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
…studio cards + spacing with Figma
- Remove the redundant "Run an optimization" h2 above the studio cards
- Fix header/cards/separator spacing to match the Figma runs-list layout
- Align the studio card title with Figma ("Use the Optimization studio")
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| <div className="grid grid-cols-1 gap-3 md:grid-cols-3"> | ||
| {cards.map(({ id, icon: Icon, title, description, onClick }) => { | ||
| const { chipColor, tintBg, tintBorder, actionLabel } = CARD_STYLES[id]; | ||
| return ( | ||
| <Button | ||
| key={id} | ||
| variant="outline" | ||
| onClick={onClick} | ||
| className="h-auto items-start justify-start gap-2 whitespace-normal px-3 pb-2 pt-3 text-left shadow-sm transition-shadow hover:shadow-md" | ||
| style={{ backgroundColor: tintBg, borderColor: tintBorder }} | ||
| > | ||
| <span |
There was a problem hiding this comment.
cards.map returns <Button>s without a key, so React can't reconcile instances across rerenders and may lose focus/hover state — should we add key={id} to the root Button?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
In
apps/opik-frontend/src/v2/pages-shared/optimizations/StudioTemplates/StudioTemplates.tsx
around lines 51-63, the `cards.map(({ id, icon: Icon, title, description, onClick }) =>
...)` callback renders a `<Button>` for each card without a stable React `key`. Add
`key={id}` to the root `Button` element returned from the map, using the existing `id`
from the destructured card data. This removes React's key warning and ensures correct
reconciliation for focus/hover state during rerenders.
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
|
🌙 Nightly cleanup: The test environment for this PR ( |
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
|
🌙 Nightly cleanup: The test environment for this PR ( |
|
🌙 Nightly cleanup: The test environment for this PR ( |
2 similar comments
|
🌙 Nightly cleanup: The test environment for this PR ( |
|
🌙 Nightly cleanup: The test environment for this PR ( |
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
|
❌ Test environment deployment failed The deployment encountered an error. Please check the deployment logs for details. |
1 similar comment
|
❌ Test environment deployment failed The deployment encountered an error. Please check the deployment logs for details. |
|
✅ Test environment is now available! Access Information
The environment was rebuilt after its EBS volumes were reaped by the nightly cleanup (all 9 pods now |
|
🌙 Nightly cleanup: The test environment for this PR ( |
Resolves conflicts introduced by PR #7227 (Optimization runs list). All four conflicts + one silent semantic conflict were in the optimization FE area: - lib/optimizations.ts: keep both new helpers (getMetricLabel from #7310, getOptimizationOptimizerType from #7227). - PercentageTrend.tsx: adopt #7227's getConfig -> getTrendConfig rename; keep #7310's tagVariant cast used by the compact renderTag(). - optimizationChartUtils.ts: keep the shared "Discarded" label + #7227's clarifying comment; #7310's fuchsia color scale is preserved. - OptimizationHeader.tsx: keep #7310's redesigned header + inline status pill (per decision); the shared OptimizationStatusTag stays in use on the runs list. - OptimizationsColumns.tsx (#7227): repoint getMetricLabel import to its new canonical home @/lib/optimizations (was @/lib/optimization-config). Verified: tsc --noEmit clean, eslint clean, 238 optimization unit tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Details
Aligns the v2 Optimization runs list to the Figma redesign (562:37189) and folds in the list-page refactors. Part of OPIK_6528. v2 only; no backend, no migrations, no
v1changes.Design
getOptimizerLabel(studio_config.optimizer.type)) and Metric (objective_name); the old "Dataset name" column now reuses the ExperimentsItemSourceCellrelabelled "Item source" (so it gets the icon + tooltip for free and matches Experiments). Per the Figma, the default-visible set is Name, Start time, Status, Pass rate, Accuracy, Latency, Cost, Opt. cost — Run ID / Item source / Algorithm / Metric ship available-but-hidden (enable from the Columns picker).prunedkey is unchanged). Renders on the trials/chart surface (PR 5), done here per the ticket.Fold-in refactors (v2 only)
OptimizationsPageintoOptimizationsColumns(column defs) +OptimizationsToolbar(search/filters/actions/refresh/columns) + a slim page.deploycolumn (referenced in the order/selected lists but never defined) and bumped the column-prefs keysv1 → v2so stale prefs don't carry the dead id.pollWhileInProgressparam to the shareduseOptimizationsView(default keeps the old unconditional 30s for v1); v2 now polls only while a run isrunning/initializedand stops once everything settles.DatasetNameCell.Notes for reviewers
ItemSourceCellchanges are additive: (1) an optionalcustomMeta.resourceoverride (used here to force the test-suite link/icon, since optimization rows carry noevaluation_method); (2) an idless fallback — when a row has a source name but no linkable id, the cell now renders the name (no link) instead of"-"(was a regression vs the oldDatasetNameCell; benefits Experiments too). Experiments passes neither new option, and the only behavior change there is idless rows showing a name instead of a dash.as nevercasts" — these exist becauseColumnData.cell(intypes/shared.ts) is typed as tanstack'sCell<T>(cell instance) instead of a cell renderer, so ~100 files app-wide useas never. Removing properly means re-typing the shared field; out of scope, tracked separately. Kept the casts (with an explanatory comment) for consistency.PercentageTrend/MetricComparisonCell. Deferred pending an exact color from design.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
ItemSourceCellreuse. Column order/visibility matched to Figma via the Figma MCP (562:37189, header 686:35271).Testing
Commands (from
apps/opik-frontend):tsc --project tsconfig.json --noEmit— clean.eslinton all changed files — 0 problems.vitest runover the optimization suites — 32 tests pass (regression:optimizationChartUtils,sortCandidates,OptimizationCompareRedirect), confirming the status-label and column changes don't break existing logic.No new unit tests were added: PR 2 is UI config + a refactor with no substantial new pure logic to test (the polling predicate is a trivial status check). The runtime UI was not exercised in a running app here — list rendering can be confirmed during review.
Documentation
N/A — UI-only changes (columns, labels, polling); no documentation update.
🤖 Generated with Claude Code