[OPIK-6167] [FE] fix: rename Test suite column to Item source#6504
[OPIK-6167] [FE] fix: rename Test suite column to Item source#6504
Conversation
…h dynamic icon and group label Unifies the experiment list dataset/test-suite column to "Item source", showing a Database/ListChecks icon per row and a dynamic "Dataset"/"Test suite" prefix on grouped rows; v2 only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…experiments tab, and dashboard widgets Apply the rename and icon badge to remaining v2 surfaces missed in the first pass: experiments-grouped chart legend, Prompt page experiments tab, ExperimentWidgetDataSection filter list, and the leaderboard widget cell. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| const datasetTypeMap = useMemo<Record<string, DATASET_TYPE>>(() => { | ||
| const map: Record<string, DATASET_TYPE> = {}; | ||
| datasetsData?.content?.forEach((dataset) => { | ||
| if (dataset.type) { | ||
| map[dataset.id] = dataset.type; |
There was a problem hiding this comment.
datasetTypeMap is only built from datasetsData.content (page 1 with size: MAX_ENTITIES_FOR_SORTING), so for datasets outside that slice useExperimentsTableConfig falls back to "Dataset" instead of "Test suite" — should we hydrate the map across all pages or include the dataset type on the grouped rows?
Finding type: Logical Bugs | Severity: 🟢 Low
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/useGroupedExperimentsList.ts around lines 485-493 where
`datasetTypeMap` is created from `datasetsData?.content`, fix the logical bug caused by
only fetching page 1 (`useDatasetsList` with `page: 1` and `size:
MAX_ENTITIES_FOR_SORTING`). Refactor so `datasetTypeMap` is populated for all dataset
ids that appear in the grouped rows—either by fetching all dataset pages (or at least
all pages needed for `flattenDeepestGroups`) to build a complete map, or by
carrying/deriving the dataset type directly when building grouped rows (e.g., include
dataset type on the group data / flatten step) so the table config doesn’t depend on a
truncated dataset slice. Update the dependencies accordingly and ensure the final
`transformedData` includes the complete `datasetTypeMap` for all grouped dataset rows.
There was a problem hiding this comment.
Made less critical in 2cc586e. The grouped row label and the resource-link routing now both read evaluation_method from the row first and only fall back to datasetTypeMap when the field is missing. Since the API returns evaluation_method on every experiment row, the page-1 truncation no longer affects label or routing for current-shape data; the map is now a fallback rather than the primary source of truth. Hydrating across all pages would still be a defensible improvement, but is no longer a correctness issue.
There was a problem hiding this comment.
Thanks, that makes sense — with evaluation_method now read from the row first, this is no longer a correctness issue for current data. I’ll save this to memory once the PR is merged.
There was a problem hiding this comment.
Commit 2cc586e addressed this comment by checking evaluation_method on the grouped rows when configuring the dataset column so we can label and link to them as Test suite or Dataset before resorting to the limited datasetTypeMap, eliminating the need to hydrate the map with every page.
| getGroupRowLabel: (row: T) => { | ||
| const datasetId = get(row, `${metaKey}.value`, "") as string; | ||
| const type = datasetTypeMap?.[datasetId]; | ||
| return type === DATASET_TYPE.TEST_SUITE | ||
| ? "Test suite" | ||
| : "Dataset"; |
There was a problem hiding this comment.
getGroupRowLabel defaults missing datasetTypeMap entries to "Dataset", so callers that omit the prop (e.g. PromptPage/ExperimentsTab) or have deleted/orphan groups lose their original type label — should we carry the dataset type in the grouped metadata directly, or make datasetTypeMap required and pass it through all grouped-experiments callers?
Finding types: Logical Bugs | Severity: 🔴 High
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/useExperimentsTableConfig.tsx`
around lines 258-263, inside the `COLUMN_DATASET_ID` `customMeta.getGroupRowLabel`
logic, stop defaulting all missing `datasetTypeMap[datasetId]` lookups to `"Dataset"`.
Preferred fix: embed the dataset type into the grouped row metadata when building the
`COLUMN_DATASET_ID` group cell, and have `getGroupRowLabel` read it from there via
`generateGroupedRowCellDef`, only falling back when the type is genuinely known.
Fallback fix: make `datasetTypeMap` required for dataset grouping and update all
grouped-experiments callers (notably `PromptPage/ExperimentsTab`) to pass it through.
Either way, callers that omit `datasetTypeMap` or reference deleted/orphan datasets must
no longer silently render grouped rows as `"Dataset"` by default.
There was a problem hiding this comment.
Commit 4039975 addressed this comment by threading a datasetTypeMap through the grouped-experiments pipeline: useGroupedExperimentsList now builds the map, the Prompt and General tabs pass it into useExperimentsTableConfig, and the dataset grouping column now uses the ItemSourceCell. customMeta.getGroupRowLabel now only emits “Test suite”/“Dataset” when the map has a known type, and the shared generateGroupedRowCellDef helper renders that resolved label (falling back to the new “Item source” heading) instead of defaulting every missing entry to “Dataset.”
| const { custom } = context.column.columnDef.meta ?? {}; | ||
| const cellData = context.row.original; | ||
| const { | ||
| resource, | ||
| nameKey = "name", |
There was a problem hiding this comment.
ItemSourceCell duplicates ResourceCell’s metadata extraction and then renders ResourceLink with the same props, so future ResourceCell changes won’t carry over—should we reuse ResourceCell (e.g., wrap it to add the icon) instead of copying its logic?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
There was a problem hiding this comment.
Intentionally not addressed. ItemSourceCell needs to render an icon adjacent to the link and to derive the resource type from evaluation_method, neither of which ResourceCell exposes. Wrapping ResourceCell would require either widening its API to render arbitrary leading content or composing a parent that re-extracts the same row metadata anyway. The current duplication is small (~10 lines of get calls) and keeps both components focused; reusing ResourceCell here would couple it to experiment-specific concerns.
There was a problem hiding this comment.
Thanks, that makes sense — the duplication here is small and avoiding coupling ResourceCell to experiment-specific behavior is a good reason to keep ItemSourceCell separate. I'll save this to memory once the PR is merged.
| import DataTableNoData from "@/shared/DataTableNoData/DataTableNoData"; | ||
| import DataTableVirtualBody from "@/shared/DataTable/DataTableVirtualBody"; | ||
| import ResourceCell from "@/shared/DataTableCells/ResourceCell"; | ||
| import ItemSourceCell from "@/v2/pages/ExperimentsPage/GeneralDatasetsTab/ItemSourceCell"; |
There was a problem hiding this comment.
PromptPage imports ItemSourceCell from v2/pages/ExperimentsPage/..., which is a disallowed cross-page dependency, so should we move ItemSourceCell to shared/ or v2/pages-shared/ and use it from both pages?
Finding type: AI Coding Guidelines | Severity: 🟢 Low
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/PromptPage/ExperimentsTab/ExperimentsTab.tsx around
lines 23 and the `columnsDef` memo where the “Item source” column uses `cell:
ItemSourceCell`, you currently import ItemSourceCell from
`@/v2/pages/ExperimentsPage/GeneralDatasetsTab/ItemSourceCell`, which violates the “no
cross-page imports” guideline. Fix this by moving `ItemSourceCell` into a shared
location that can be consumed by both pages (e.g., `@/shared/...` or
`@/v2/pages-shared/...`), then update both PromptPage and ExperimentsPage to import it
from the new shared path and remove the old page-local import.
There was a problem hiding this comment.
Commit 4039975 addressed this comment by relocating ItemSourceCell into the shared v2/pages-shared/experiments directory and updating both PromptPage and ExperimentsPage to import the cell from that shared path, eliminating the disallowed cross-page dependency.
There was a problem hiding this comment.
Commit 4f39e97 addressed this comment by moving ItemSourceCell into @/v2/pages-shared/experiments/ItemSourceCell and updating the PromptPage's ExperimentsTab to import from the shared location instead of the ExperimentsPage-only path. The ExperimentsPage's GeneralDatasetsTab now also imports the shared ItemSourceCell and the new column label ensures consistent usage without cross-page dependency.
| }, | ||
| { | ||
| id: COLUMN_DATASET_ID, | ||
| label: "Test suite", | ||
| label: "Item source", | ||
| type: COLUMN_TYPE.string, | ||
| cell: ResourceCell as never, | ||
| cell: ItemSourceCell as never, | ||
| customMeta: { | ||
| nameKey: "dataset_name", | ||
| idKey: "dataset_id", |
There was a problem hiding this comment.
The dataset column repeats the same ItemSourceCell + customMeta in multiple files, so should we extract a shared ITEM_SOURCE_COLUMN constant/helper and reuse it instead of copy-pasting?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
There was a problem hiding this comment.
Partially addressed. ITEM_SOURCE_LABEL is now a shared constant (exported from v2/pages-shared/experiments/ItemSourceCell), so the column label is no longer duplicated. The remaining customMeta for the dataset_id column already differs between ExperimentsTab (no datasetTypeMap, no group-by) and GeneralDatasetsTab (full grouping with datasetTypeMap/getResource), so an ITEM_SOURCE_COLUMN factory would need enough config knobs to cover both shapes — which would re-introduce most of the per-caller config it tries to eliminate. Leaving the column-level config inline keeps each callers tab obvious about what it supports.
There was a problem hiding this comment.
Thanks, that makes sense — the shared label constant addresses the real duplication, and keeping the remaining column config inline preserves each tab’s intent. I’ll save this to memory once the PR is merged.
There was a problem hiding this comment.
Commit 98e05eb addressed this comment by centralizing the item-source metadata: the new resolveItemSourceResource helper and shared ITEM_SOURCE_LABEL now keep both tables aligned on the same column semantics without copy-pasting the ItemSourceCell/customMeta plumbing, while still allowing each tab to keep any extra knobs inline.
| import CostCell from "@/shared/DataTableCells/CostCell"; | ||
| import PassRateCell from "@/shared/DataTableCells/PassRateCell"; | ||
| import ResourceCell from "@/shared/DataTableCells/ResourceCell"; | ||
| import ItemSourceCell from "@/v2/pages/ExperimentsPage/GeneralDatasetsTab/ItemSourceCell"; |
There was a problem hiding this comment.
helpers.ts imports ItemSourceCell from @/v2/pages/ExperimentsPage/..., violating the no cross-page import rule — should we move ItemSourceCell into a shared location (e.g. v2/pages-shared/ or shared/) and update all consumers, or move the ExperimentsPage-specific column config out of this shared helper entirely?
Finding types: Avoid tight coupling AI Coding Guidelines | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In
apps/opik-frontend/src/v2/pages-shared/dashboards/widgets/ExperimentsLeaderboardWidget/helpers.ts
around lines 17 and 60, remove the page-specific dependency on
`@/v2/pages/ExperimentsPage/GeneralDatasetsTab/ItemSourceCell`. Refactor by either: (a)
Moving `ItemSourceCell` into a shared module (e.g.
`apps/opik-frontend/src/v2/pages-shared/...` or `apps/opik-frontend/src/shared/...`),
updating barrel exports if needed, and changing the import in helpers.ts and any other
consumers to use the new shared path, or (b) Relocating the dataset/item-source column
configuration (including the `cell: ItemSourceCell as never` usage) out of this shared
`helpers.ts` so it lives within the ExperimentsPage scope where the component dependency
is appropriate. Ensure the shared dashboard helpers no longer import from
ExperimentsPage paths, the moved component remains functionally identical, and the
build/typecheck passes.
There was a problem hiding this comment.
Commit 4039975 addressed this comment by moving ItemSourceCell into the shared v2/pages-shared/experiments directory and updating the ExperimentsLeaderboardWidget helper (and other consumers such as GeneralDatasetsTab) to import it from the shared path, eliminating the cross-page dependency.
There was a problem hiding this comment.
Commit 4f39e97 addressed this comment by moving ItemSourceCell into the shared v2/pages-shared/experiments directory and updating helpers.ts (and the ExperimentsPage consumer) to import from that shared module, removing the cross-page dependency.
| const groupField = groups[index]?.field; | ||
| const groupLabel = | ||
| groupField === COLUMN_DATASET_ID | ||
| ? "Item source" | ||
| : calculateGroupLabel(groups[index]); | ||
|
|
||
| return { | ||
| label: calculateGroupLabel(groups[index]), | ||
| label: groupLabel, | ||
| value: | ||
| label === DELETED_ENTITY_LABEL | ||
| ? "Deleted test suite" |
There was a problem hiding this comment.
The "Item source" label is hardcoded here and duplicated across ExperimentsTab.tsx and ExperimentWidgetDataSection.tsx, and the deleted-entity branch still hardcodes value: "Deleted test suite" — should we extract a shared ITEM_SOURCE_LABEL constant and align the deleted-entity value with the renamed dataset/item-source semantics?
Finding types: Extract naming constants Logical Bugs | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
In GeneralDatasetsTab.tsx around lines 572-582, inside the `deepestExpandedGroups`
useMemo map: 1. Replace the hardcoded `"Item source"` string with a shared constant
(e.g., `ITEM_SOURCE_LABEL`), and update all other occurrences in `ExperimentsTab.tsx`
(around lines 209-214) and `ExperimentWidgetDataSection.tsx` (around lines 41-46) to
import and reuse the same constant. Add it to an appropriate shared constants/util
module if needed. 2. In the deleted-entity branch where `label ===
DELETED_ENTITY_LABEL`, replace the hardcoded `value: "Deleted test suite"` with the
correct value aligned to the renamed dataset/item-source semantics (consistent with the
non-deleted path and the `groupLabel`/`groupField === COLUMN_DATASET_ID` naming).
There was a problem hiding this comment.
Commit 4039975 addressed this comment by deriving the dataset-id group label as “Item source” and by updating the deleted-entity value to “Deleted dataset,” aligning the deleted branch with the renamed dataset/item-source semantics (the shared constant extraction is still pending).
There was a problem hiding this comment.
Commit 4f39e97 addressed this comment by updating the deleted-entity branch to return “Deleted dataset” and by ensuring dataset groups render the dataset-specific “Item source” label, aligning the new dataset/item-source semantics with the grouped rows.
…eleted-entity label Resolves frontend dependency-cruiser violations: ItemSourceCell no longer lives under a page module imported by another page or by pages-shared helpers. Also retires the stale "Deleted test suite" string from the chart legend in favor of a neutral "Deleted dataset" matching the underlying entity. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… unknown in group row label Avoids confidently mislabeling unknown/deleted/orphan groups as "Dataset" when the dataset is not in the loaded datasets slice. Returning an empty string lets generateGroupedRowCellDef fall back to the static column label, which is neutral and correct. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| <ResourceLink | ||
| id={id} | ||
| name={name} | ||
| resource={resource} | ||
| isDeleted={isDeleted} |
There was a problem hiding this comment.
evaluation_method === EVALUATION_METHOD.TEST_SUITE only changes the icon, but ResourceLink still uses resource with callers hardcoding RESOURCE_TYPE.dataset, so test-suite routes end up with suiteId under a dataset path—should we switch the resource/RESOURCE_TYPE based on evaluation_method?
Finding type: Type Inconsistency | Severity: 🔴 High
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/experiments/ItemSourceCell.tsx around lines 50-54
inside the `ItemSourceCell` render, the icon switches on `evaluation_method` but the
`ResourceLink` still uses the column meta `resource`, which results in dataset routing
when `evaluation_method === EVALUATION_METHOD.TEST_SUITE`. Refactor the component to
also select the `resource` passed to `ResourceLink` based on `evaluationMethod` (e.g.,
use `RESOURCE_TYPE.testSuite` for `TEST_SUITE`, otherwise fall back to the existing
`resource` from `custom`/meta). If you prefer not to hardcode, update the column meta
typing/usage so callers can pass an explicit test-suite resource override and ensure
`ItemSourceCell` uses it when `evaluation_method` indicates a test suite.
There was a problem hiding this comment.
Fixed in 2cc586e. ItemSourceCell now derives both the icon and the RESOURCE_TYPE from evaluation_method directly (TEST_SUITE → RESOURCE_TYPE.testSuite, otherwise dataset). Removed the unused resource from the cell meta and dropped the now-redundant resource: RESOURCE_TYPE.dataset from all callers. The same fix was applied to the grouped variant in useExperimentsTableConfig.tsx via a new getResource callback on ResourceCell.Group, falling back to datasetTypeMap when the row lacks evaluation_method.
There was a problem hiding this comment.
Thanks for the update — that makes sense. I’ll save this to memory once the PR is merged.
There was a problem hiding this comment.
Commit 2cc586e addressed this comment by removing the hardcoded resource metadata and deriving both the icon and ResourceLink resource from evaluation_method, so test-suite rows now pass RESOURCE_TYPE.testSuite (with datasets kept in their own branch). This ensures the link targets the correct route whenever the evaluation is a test suite.
…bel to match Figma Figma OPIK-6167 shows the prefix as "Test suite test_suite_name (23)" (no colon between the label and the linked entity). Add an additive hideGroupRowLabelColon flag in customMeta and set it for the dataset group case in v2; v1 group rows keep their existing colon since they do not pass the flag. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…on evaluation_method
- ItemSourceCell: derive resource (and icon) from evaluation_method so
TEST_SUITE experiments link to /test-suites/{suiteId}/items instead of
the dataset URL; drop now-redundant resource field from customMeta.
- ResourceCell.Group: add optional getResource(cellData) callback so
callers can pick the link's resource at render time.
- useExperimentsTableConfig: wire getResource on the dataset_id group
cell to switch to RESOURCE_TYPE.testSuite when the row is a test
suite, and read evaluation_method on the row directly in
getGroupRowLabel before falling back to datasetTypeMap (covers
datasets beyond the page-1 slice fetched by useDatasetsList).
- Extract ITEM_SOURCE_LABEL constant from ItemSourceCell and reuse it
across the experiments table, filters/groups config, dashboard
widgets, and prompt experiments tab to avoid drift.
| const datasetId = get(row, `${metaKey}.value`, "") as string; | ||
| if (datasetTypeMap?.[datasetId] === DATASET_TYPE.TEST_SUITE) | ||
| return RESOURCE_TYPE.testSuite; | ||
| return RESOURCE_TYPE.dataset; |
There was a problem hiding this comment.
datasetTypeMap is only filled from the first 1000 datasets in useGroupedExperimentsList, so missing entries get treated as RESOURCE_TYPE.dataset and getResource can misclassify grouped rows with absent evaluation_method; should we fetch the resource type from the grouped row/backend instead of a partial client-side map?
Finding type: Type Inconsistency | Severity: 🟠 Medium
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/experiments/useExperimentsTableConfig.tsx around
lines 261-264 inside the group column CASE COLUMN_DATASET_ID customMeta `getResource`
(and the related `getGroupRowLabel` block around lines 274-289), the code derives
resource type from `datasetTypeMap?.[datasetId]` and then defaults to
RESOURCE_TYPE.dataset when the map is missing due to only being built from the first
1000 datasets. Refactor this so it does not depend on the partial client-side
`datasetTypeMap` for correctness when `evaluation_method` is absent—source the dataset
type from the grouped row/backend response (preferred) by adding/using a field like
dataset_type/resource_type in the grouped experiments list, or otherwise make the
fallback explicit (return an unknown/undefined resource type and ensure
ResourceLink/group label doesn’t mis-route). Also deduplicate the
evaluation_method/type-to-label logic so `getResource` and `getGroupRowLabel` use the
same helper and the same source of truth.
There was a problem hiding this comment.
Made the fallback explicit in 98e05eb. getResource and getGroupRowLabel now share a single resolver (resolveItemSourceResource) that returns undefined when neither evaluation_method nor datasetTypeMap[dataset_id] resolves to a known type. The group cell (ResourceCell.Group) renders the name as plain text without a link in that case instead of routing to /datasets/:id/items. Group label falls back to "Item source" rather than "Dataset". Hydrating datasetTypeMap across all dataset pages would still be a defensible improvement (or surfacing evaluation_method directly on the grouped row response), but the silent mis-route is gone — when we cant prove the type, we dont link.
There was a problem hiding this comment.
Thanks, that makes sense — the explicit unknown-type fallback removes the silent mis-route concern. I'll save this to memory once the PR is merged.
There was a problem hiding this comment.
Commit 98e05eb addressed this comment by introducing resolveItemSourceResource so both the group label and ResourceLink logic now derive the resource type from the row’s evaluation_method when present before falling back to the datasetTypeMap, eliminating the misclassification caused by the partial map and ensuring grouped rows without evaluation_method stay unrouted rather than defaulting to DATASET.
| const isTestSuite = evaluationMethod === EVALUATION_METHOD.TEST_SUITE; | ||
| const Icon = isTestSuite ? ListChecks : Database; | ||
| const resource = isTestSuite | ||
| ? RESOURCE_TYPE.testSuite | ||
| : RESOURCE_TYPE.dataset; |
There was a problem hiding this comment.
evaluation_method only gates the test-suite case, so missing/undefined rows get resolved via the dataset path (show dataset icon/link and go to /datasets/:datasetId/items) instead of /test-suites/:suiteId/items; should we resolve the resource like useExperimentsTableConfig or pass a precomputed resource?
Finding type: Type Inconsistency | Severity: 🟠 Medium
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/experiments/ItemSourceCell.tsx around lines 39-43
(the component logic that sets `isTestSuite`, `Icon`, and `resource`), the code treats a
missing/undefined `evaluation_method` as “not test suite” and always falls back to
`RESOURCE_TYPE.dataset`. Refactor this so the resource/icon selection uses the same
fallback strategy as `useExperimentsTableConfig` (i.e., determine the correct resource
type when `Experiment.evaluation_method` is optional/missing) or change the component
API to accept a precomputed `resource` from the parent/table config. After the change,
ensure rows that represent test suites navigate to `/test-suites/:suiteId/items` rather
than `/datasets/:datasetId/items` when `evaluation_method` is absent.
There was a problem hiding this comment.
Aligned with useExperimentsTableConfig in 98e05eb. ItemSourceCell now reads datasetTypeMap from tableMeta and uses the shared resolveItemSourceResource resolver: evaluation_method first, then datasetTypeMap[dataset_id], otherwise undefined. When the resolver returns undefined, the cell renders the name as plain text (no link, default Database icon) rather than mis-routing to /datasets/:id/items. Added datasetTypeMap?: Record<string, DATASET_TYPE> to the TableMeta augmentation so any consumer that already has the map can plumb it through meta.
There was a problem hiding this comment.
Thanks for the clarification — that shared resolver and datasetTypeMap fallback make sense here. I'll save this to memory once the PR is merged.
There was a problem hiding this comment.
Commit 98e05eb addressed this comment by introducing resolveItemSourceResource, which mirrors useExperimentsTableConfig’s fallback and checks both evaluation_method and the datasetTypeMap stored on the table meta before deciding a RESOURCE_TYPE. ItemSourceCell now only renders the ResourceLink when this resolver returns a resource, ensuring that rows with missing evaluation_method but dataset metadata indicating a test suite still resolve to the test-suite route rather than defaulting to datasets.
…nown When an experiment row lacks evaluation_method and the dataset isn't in the (page-1-only) datasetTypeMap, ItemSourceCell and the dataset group cell were silently routing to /datasets/:id/items, which 404s for test suites. Extract a shared resolveItemSourceResource helper, plumb datasetTypeMap through TableMeta so ItemSourceCell can use the same fallback as the group cell, and render plain text (no link) when the resource type can't be determined.
| export const resolveItemSourceResource = ( | ||
| evaluationMethod: EVALUATION_METHOD | undefined, | ||
| datasetId: string | undefined, | ||
| datasetTypeMap?: Record<string, DATASET_TYPE>, | ||
| ): RESOURCE_TYPE | undefined => { |
There was a problem hiding this comment.
resolveItemSourceResource adds branching for TEST_SUITE/DATASET, datasetTypeMap fallback, and undefined, but there’s no Vitest coverage in apps/opik-frontend/src/v2/pages-shared/experiments (or helper tests elsewhere) to lock the resource/link and label mappings—should we add a small unit test suite for the helper’s branch cases?
Finding type: AI Coding Guidelines | Severity: 🟢 Low
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/experiments/ItemSourceCell.tsx around lines
15-32, the resolveItemSourceResource helper has multiple conditional branches
(TEST_SUITE, DATASET, datasetId+datasetTypeMap mapping, and undefined fallback) but
there’s no Vitest coverage to prevent regressions. Add a small Vitest test suite (per
.agents/skills/opik-frontend/testing.md) that imports resolveItemSourceResource and
asserts the returned RESOURCE_TYPE for: evaluationMethod=TEST_SUITE,
evaluationMethod=DATASET, datasetTypeMap mapping to TEST_SUITE and DATASET when
datasetId is present, and cases where datasetId/datasetTypeMap are missing or mapping
doesn’t match (expect undefined). Place the test alongside the helper (or in the
nearest existing __tests__/vitest setup for this folder) and ensure it runs in the
current frontend test config.
There was a problem hiding this comment.
Commit 8e75bf6 addressed this comment by removing resolveItemSourceResource and its branching logic from ItemSourceCell; the component now directly infers resource type without the helper, so there is no longer a helper to cover with those Vitest cases.
…e-test-suite-column-to-item-source # Conflicts: # apps/opik-frontend/src/v2/pages-shared/experiments/useExperimentsGroupsAndFilters.ts
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
…plumbing Reverts 98e05eb surgically — keeps the rename, dynamic icon, and test-suite routing from the prior commits, and drops: - DataTable TableMeta augmentation with datasetTypeMap (touched table core) - ResourceCell.Group signature widening + plain-text fallback (shared cell) - resolveItemSourceResource helper + datasetTypeMap plumbing in ItemSourceCell - shared resolver call sites in useExperimentsTableConfig Group cell falls back to RESOURCE_TYPE.dataset when the type is unknown, which can mis-route deleted/orphan rows. That's an acceptable trade-off for OPIK-6167 versus modifying core table types and shared cells.
|
✅ 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. |
…pedRowCellDef generateGroupedRowCellDef is a shared helper. getGroupRowLabel and hideGroupRowLabelColon were added solely for the experiments dataset column — neither has any other caller. They don't belong in shared table utils. Group rows now show the column's static "Item source" label with the default trailing colon, matching every other grouped table.
…amic prefix - Wrap data-cell icon in slate-100 24x24 rounded pill matching Figma node 6095:48496 - Add createItemSourceGroupCell factory owning the full group-row layout (checkbox + chevron + dynamic prefix + resource link/text + count + explainer), keeping generateGroupedRowCellDef and ResourceCell.Group untouched - Pass datasetTypeMap via column customMeta instead of TableMeta Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| if (datasetId && datasetTypeMap) { | ||
| const t = datasetTypeMap[datasetId]; | ||
| if (t === DATASET_TYPE.TEST_SUITE) { | ||
| return { prefix: "Test suite", resource: RESOURCE_TYPE.testSuite }; | ||
| } | ||
| if (t === DATASET_TYPE.DATASET) { | ||
| return { prefix: "Dataset", resource: RESOURCE_TYPE.dataset }; | ||
| } | ||
| } | ||
| return { prefix: ITEM_SOURCE_LABEL, resource: undefined }; |
There was a problem hiding this comment.
If datasetTypeMap lacks a grouped dataset_id, this falls back to ITEM_SOURCE_LABEL with resource: undefined, so ItemSourceGroupCell renders plain text instead of the dataset/test-suite ResourceLink; can we preserve the resource for the group row or make the type map exhaustive?
Finding type: Logical Bugs | Severity: 🟢 Low
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/experiments/ItemSourceCell.tsx around lines
85-105, the resolveGroupPrefix fallback returns { prefix: ITEM_SOURCE_LABEL, resource:
undefined } when datasetId is present but datasetTypeMap doesn’t include it, causing
ItemSourceGroupCell (lines 194-205) to render plain text instead of the ResourceLink for
grouped dataset/test-suite rows. Refactor resolveGroupPrefix to avoid dropping the
specific RESOURCE_TYPE when the type map is incomplete—either (a) infer the resource
from other fields already available in cellData (e.g., evaluation_method or dataset type
metadata) or (b) add a second, safer lookup/fallback so that if evaluationMethod is
DATASET you still return resource: RESOURCE_TYPE.dataset and a consistent prefix even
when datasetTypeMap lacks the id. Then adjust ItemSourceGroupCell’s conditional
rendering (lines 194-205) only if needed so id + derived resource always results in
ResourceLink for grouped rows.
| <CellWrapper | ||
| metadata={context.column.columnDef.meta} | ||
| tableMetadata={context.table.options.meta} | ||
| className="items-center p-0 pr-2" | ||
| stopClickPropagation |
There was a problem hiding this comment.
ItemSourceGroupCell duplicates the grouped-row checkbox/expand/depth padding already in generateGroupedRowCellDef in shared/DataTable/utils.tsx, so could we keep the shared grouped-row wrapper and only override the inner content (prefix/ResourceLink)?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
|
🌙 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 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. |
Co-Authored-By: Claude Opus 4.7 <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. |
|
🌙 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. |
awkoy
added
test-environment
|
🔄 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 ( |
4 participants
Details
Unifies the experiments list dataset/test-suite column header to "Item source", displays a per-row Database/ListChecks icon based on the experiment's
evaluation_method, and renders a dynamic "Dataset"/"Test suite" prefix on grouped rows so the page reads consistently regardless of which entity an experiment points at. v2 only — v1, the Trial page, and the standalone test-suite item browser are unchanged.ItemSourceCell(v2) that renders aDatabaseicon for dataset experiments and aListChecksicon for test-suite experiments alongside the existing resource link.datasetTypeMap(built fromuseDatasetsListdata inuseGroupedExperimentsList) through touseExperimentsTableConfig; used to compute the per-row group prefix label dynamically.generateGroupedRowCellDefto honor an optionalcustomMeta.getGroupRowLabel(row). v1 callers do not pass it, so v1 behavior is unchanged.generateGroupedRowCellDefand overridingenableHiding: trueat the v2 call site.Change checklist
Issues
AI-WATERMARK
AI-WATERMARK: yes
ItemSourceCellcomponentTesting
npx tsc --noEmit— passesnpx eslinton all modified files — cleanManual checks to perform after merge / preview:
Documentation
n/a — user-visible label change only.