Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ import {
buildDynamicMetadataColumns,
} from "@/lib/metadata";
import { BaseTraceData, Span, SPAN_TYPE, Trace } from "@/types/traces";
import { convertColumnDataToColumn, migrateSelectedColumns } from "@/lib/table";
import {
convertColumnDataToColumn,
migrateColumnsOrder,
migrateSelectedColumns,
} from "@/lib/table";
import { getJSONPaths } from "@/lib/utils";
import { generateSelectColumDef } from "@/components/shared/DataTable/utils";
import NoTracesPage from "@/components/pages/TracesPage/NoTracesPage";
Expand Down Expand Up @@ -236,7 +240,19 @@ const DEFAULT_TRACES_COLUMN_PINNING: ColumnPinningState = {
right: [],
};

const DEFAULT_TRACES_PAGE_COLUMNS: string[] = [
const DEFAULT_TRACES_COLUMNS: string[] = [
"start_time",
"input",
"output",
"error_info",
"duration",
"usage.total_tokens",
"total_estimated_cost",
"tags",
COLUMN_COMMENTS_ID,
];

const DEFAULT_SPANS_COLUMNS: string[] = [
COLUMN_ID_ID,
"name",
"start_time",
Expand All @@ -247,10 +263,23 @@ const DEFAULT_TRACES_PAGE_COLUMNS: string[] = [
USER_FEEDBACK_COLUMN_ID,
];

const DEFAULT_TRACES_ORDER: string[] = [
"start_time",
"input",
"output",
"error_info",
"duration",
"usage.total_tokens",
"total_estimated_cost",
"tags",
COLUMN_COMMENTS_ID,
];

const SELECTED_COLUMNS_KEY_SUFFIX = "selected-columns";
const SELECTED_COLUMNS_KEY_V2_SUFFIX = `${SELECTED_COLUMNS_KEY_SUFFIX}-v2`;
const COLUMNS_WIDTH_KEY_SUFFIX = "columns-width";
const COLUMNS_ORDER_KEY_SUFFIX = "columns-order";
const COLUMNS_ORDER_V2_KEY_SUFFIX = `${COLUMNS_ORDER_KEY_SUFFIX}-v2`;
const COLUMNS_SORT_KEY_SUFFIX = "columns-sort";
const COLUMNS_SCORES_ORDER_KEY_SUFFIX = "scores-columns-order";
const DYNAMIC_COLUMNS_KEY_SUFFIX = "dynamic-columns";
Expand Down Expand Up @@ -480,7 +509,9 @@ export const TracesSpansTab: React.FC<TracesSpansTabProps> = ({
{
defaultValue: migrateSelectedColumns(
`${type}-${SELECTED_COLUMNS_KEY_SUFFIX}`,
DEFAULT_TRACES_PAGE_COLUMNS,
type === TRACE_DATA_TYPE.traces
? DEFAULT_TRACES_COLUMNS
: DEFAULT_SPANS_COLUMNS,
[COLUMN_ID_ID, "start_time"],
),
},
Expand Down Expand Up @@ -629,9 +660,12 @@ export const TracesSpansTab: React.FC<TracesSpansTabProps> = ({
);

const [columnsOrder, setColumnsOrder] = useLocalStorageState<string[]>(
`${type}-${COLUMNS_ORDER_KEY_SUFFIX}`,
`${type}-${COLUMNS_ORDER_V2_KEY_SUFFIX}`,
{
defaultValue: [],
defaultValue: migrateColumnsOrder(
`${type}-${COLUMNS_ORDER_KEY_SUFFIX}`,
type === TRACE_DATA_TYPE.traces ? DEFAULT_TRACES_ORDER : [],
),
},
);

Expand Down
21 changes: 21 additions & 0 deletions apps/opik-frontend/src/lib/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,27 @@ export const migrateSelectedColumns = (
return defaultColumns;
};

/**
* Migrates column order from an old localStorage key.
* Preserves custom user orders (non-empty arrays) and applies
* the new default order for users who never customized.
*/
export const migrateColumnsOrder = (
oldStorageKey: string,
defaultOrder: string[],
): string[] => {
const oldData = localStorage.getItem(oldStorageKey);

if (oldData !== null) {
const parsed = JSON.parse(oldData);
if (Array.isArray(parsed) && parsed.length > 0) {
return parsed;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migrateColumnsOrder JSON.parses ${type}-columns-order without guarding, so a corrupted or manually edited value throws during TracesSpansTab initialization and the table fails to render; can we wrap this parse in a try/catch and fall back to the default order instead?

Suggested change
const oldData = localStorage.getItem(oldStorageKey);
if (oldData !== null) {
const parsed = JSON.parse(oldData);
if (Array.isArray(parsed) && parsed.length > 0) {
return parsed;
const oldData = localStorage.getItem(oldStorageKey);
if (oldData !== null) {
try {
const parsed = JSON.parse(oldData);
if (Array.isArray(parsed) && parsed.length > 0) {
return parsed;
}
} catch {
// Ignore malformed JSON and fall back to defaultOrder
}

Finding type: Logical Bugs


  • Apply fix with Baz

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit 7d7652f addressed this comment by wrapping the JSON.parse(oldData) call in a try/catch inside migrateColumnsOrder. On parse failure it now ignores the malformed value and falls back to defaultOrder, preventing TracesSpansTab initialization from crashing due to corrupted localStorage.

}
}

return defaultOrder;
};

/**
* Determines if a column can be sorted based on the backend's sortable_by response.
* Handles multiple matching patterns:
Expand Down