From 5ede27e8fb40d1b13fcb97a8683386d753423a4b Mon Sep 17 00:00:00 2001 From: Ashwin Bhatkal Date: Mon, 27 Jul 2026 18:59:40 +0530 Subject: [PATCH 1/5] fix(ai-assistant): page context for the V2 panel editor (edit + create) and the v2 dashboards list in the picker (#12291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ai-assistant): add page context for the V2 panel editor route The V2 panel editor lives on `/dashboard/:dashboardId/panel/:panelId`, which `getAutoContexts` never matched — the three existing dashboard matchers are all `exact`, so a 4-segment URL fell through every branch and the assistant opened with no context chip and a page type of `other`. A saved panel maps to `panel_edit` with its id. The unsaved `panel/new` route has no id to attach yet and the schema requires a non-empty `panel_edit.widgetId`, so it degrades to the dashboard context; the in-progress query and time range still ride along in shared metadata. * fix(ai-assistant): use the v2 dashboards list in the context picker `GET /api/v1/dashboards` is now a stub that returns a V1-deprecated error, and `useGetAllDashboard` routes errors through `useErrorModal` — so opening the picker on the Dashboards tab both showed "Failed to load dashboards" and popped a global error modal. Switch to `useListDashboardsForUserV2`, the same source the V2 list page and export picker already use. `resolveAutoContextName` reads the v2 query key from cache too, so auto-context chips resolve to real dashboard titles again instead of falling back to a generic label. The assistant was the last live caller of the v1 list: the only other one, `container/ListOfDashboard`, is unreachable now that the routed `pages/DashboardsListPage` renders the V2 page. * fix(ai-assistant): report panel_create on the unsaved panel editor `/dashboard/:id/panel/new` previously reported `dashboard_detail`, so the assistant was told the user was looking at a dashboard rather than creating a panel. It now reports `panel_create`, which renders a "New panel" chip. `PageTypeDTO` has no `panel_create` member — `alert_new` is the precedent it's missing — so `resolvePageType` maps it to `panel_edit` until the backend adds one. An unmapped key would fall through to `other` and lose the empty-state chips. The context itself carries no `widgetId`, which the schema only requires when `metadata.page` is literally `panel_edit`. Also drops the `DASHBOARD_WIDGET` matcher. That V1 route is unreachable — nothing generates a link to it since V2 always builds panel-editor paths — and it read the `:widgetId` path segment, which V1's own page ignored in favour of `?widgetId=`, so its `widgetId` was never right. --- .../__tests__/getAutoContexts.test.ts | 50 ++++++++++++++++++ .../__tests__/resolvePageType.test.ts | 22 ++++++++ .../components/ChatInput/ChatInput.tsx | 51 +++++++++++++------ .../__tests__/ChatInput.prefill.test.tsx | 7 +-- .../container/AIAssistant/getAutoContexts.ts | 22 ++++---- .../container/AIAssistant/resolvePageType.ts | 2 + 6 files changed, 126 insertions(+), 28 deletions(-) diff --git a/frontend/src/container/AIAssistant/__tests__/getAutoContexts.test.ts b/frontend/src/container/AIAssistant/__tests__/getAutoContexts.test.ts index 709872e943a..1a9c37c7918 100644 --- a/frontend/src/container/AIAssistant/__tests__/getAutoContexts.test.ts +++ b/frontend/src/container/AIAssistant/__tests__/getAutoContexts.test.ts @@ -167,6 +167,56 @@ describe('getAutoContexts', () => { ]); }); + it('returns panel edit context on the V2 panel editor', () => { + const dashboardId = 'dash-123'; + const panelId = 'panel-abc'; + const pathname = ROUTES.DASHBOARD_PANEL_EDITOR.replace( + ':dashboardId', + dashboardId, + ).replace(':panelId', panelId); + + const contexts = getAutoContexts(pathname, ''); + + expect(contexts).toStrictEqual([ + { + source: 'auto', + type: 'dashboard', + resourceId: dashboardId, + metadata: { + page: 'panel_edit', + widgetId: panelId, + }, + }, + ]); + }); + + it('returns new panel context on the unsaved new-panel editor', () => { + const dashboardId = 'dash-123'; + const pathname = ROUTES.DASHBOARD_PANEL_EDITOR.replace( + ':dashboardId', + dashboardId, + ).replace(':panelId', 'new'); + const startTime = '1700000000000'; + const endTime = '1700003600000'; + + const contexts = getAutoContexts( + pathname, + `?panelKind=TimeSeries&${QueryParams.startTime}=${startTime}&${QueryParams.endTime}=${endTime}`, + ); + + expect(contexts).toStrictEqual([ + { + source: 'auto', + type: 'dashboard', + resourceId: dashboardId, + metadata: { + page: 'panel_create', + timeRange: { start: Number(startTime), end: Number(endTime) }, + }, + }, + ]); + }); + it('returns empty array on alert overview without ruleId', () => { const contexts = getAutoContexts(ROUTES.ALERT_OVERVIEW, ''); diff --git a/frontend/src/container/AIAssistant/__tests__/resolvePageType.test.ts b/frontend/src/container/AIAssistant/__tests__/resolvePageType.test.ts index e4f90e2cdb4..0e8004825da 100644 --- a/frontend/src/container/AIAssistant/__tests__/resolvePageType.test.ts +++ b/frontend/src/container/AIAssistant/__tests__/resolvePageType.test.ts @@ -17,6 +17,28 @@ describe('resolvePageType', () => { expect(resolvePageType(pathname, '')).toBe(PageTypeDTO.dashboard_detail); }); + it('returns panel_edit on the V2 panel editor', () => { + const pathname = ROUTES.DASHBOARD_PANEL_EDITOR.replace( + ':dashboardId', + 'dash-123', + ).replace(':panelId', 'panel-abc'); + + expect(resolvePageType(pathname, '')).toBe(PageTypeDTO.panel_edit); + }); + + // `panel_create` has no PageTypeDTO counterpart yet, so it reports + // `panel_edit` rather than degrading to `other`. + it('returns panel_edit on the unsaved new-panel editor', () => { + const pathname = ROUTES.DASHBOARD_PANEL_EDITOR.replace( + ':dashboardId', + 'dash-123', + ).replace(':panelId', 'new'); + + expect(resolvePageType(pathname, '?panelKind=TimeSeries')).toBe( + PageTypeDTO.panel_edit, + ); + }); + it('returns alerts_triggered on alert history without ruleId', () => { expect(resolvePageType(ROUTES.ALERT_HISTORY, '')).toBe( PageTypeDTO.alerts_triggered, diff --git a/frontend/src/container/AIAssistant/components/ChatInput/ChatInput.tsx b/frontend/src/container/AIAssistant/components/ChatInput/ChatInput.tsx index bbfd7d75d4d..6b6128e3733 100644 --- a/frontend/src/container/AIAssistant/components/ChatInput/ChatInput.tsx +++ b/frontend/src/container/AIAssistant/components/ChatInput/ChatInput.tsx @@ -16,17 +16,22 @@ import { TooltipSimple } from '@signozhq/ui/tooltip'; import type { UploadFile } from 'antd'; import getSessionStorage from 'api/browser/sessionstorage/get'; import setSessionStorage from 'api/browser/sessionstorage/set'; +import { + getListDashboardsForUserV2QueryKey, + useListDashboardsForUserV2, +} from 'api/generated/services/dashboard'; import { getListRulesQueryKey, useListRules, } from 'api/generated/services/rules'; -import type { ListRules200 } from 'api/generated/services/sigNoz.schemas'; +import type { + DashboardtypesListedDashboardForUserV2DTO, + ListDashboardsForUserV2200, + ListDashboardsForUserV2Params, + ListRules200, +} from 'api/generated/services/sigNoz.schemas'; import logEvent from 'api/common/logEvent'; -import { REACT_QUERY_KEY } from 'constants/reactQueryKeys'; -import { useGetAllDashboard } from 'hooks/dashboard/useGetAllDashboard'; import { useQueryService } from 'hooks/useQueryService'; -import type { SuccessResponseV2 } from 'types/api'; -import type { Dashboard } from 'types/api/dashboard/getAll'; // eslint-disable-next-line import { useSelector } from 'react-redux'; import { AppState } from 'store/reducers'; @@ -100,6 +105,8 @@ function autoContextLabel(ctx: MessageContext): string { return 'Current dashboard'; case 'panel_edit': return 'Editing panel'; + case 'panel_create': + return 'New panel'; case 'panel_fullscreen': return 'Panel (fullscreen)'; case 'dashboard_list': @@ -164,6 +171,18 @@ const TEXTAREA_MAX_HEIGHT_PX = 200; const HOME_SERVICES_INTERVAL = 30 * 60 * 1000; /** sessionStorage key for the "voice input failed this tab" flag. */ const VOICE_UNAVAILABLE_KEY = 'ai-assistant-voice-unavailable'; +/** + * The picker filters client-side, so it pulls one large page instead of + * paginating. Shared with `getQueryData` below — the params are part of the + * generated query key, so both sides must use the same object. + */ +const DASHBOARD_LIST_PARAMS: ListDashboardsForUserV2Params = { limit: 1000 }; + +function dashboardTitle( + dashboard: DashboardtypesListedDashboardForUserV2DTO, +): string { + return dashboard.spec.display?.name || dashboard.name || 'Untitled'; +} interface SelectedContextItem { category: ContextCategory; @@ -716,9 +735,11 @@ export default function ChatInput({ data: dashboardsResponse, isLoading: isDashboardsLoading, isError: isDashboardsError, - } = useGetAllDashboard({ - enabled: isContextPickerOpen && activeContextCategory === 'Dashboards', - staleTime: Infinity, + } = useListDashboardsForUserV2(DASHBOARD_LIST_PARAMS, { + query: { + enabled: isContextPickerOpen && activeContextCategory === 'Dashboards', + staleTime: Infinity, + }, }); const { @@ -765,12 +786,12 @@ export default function ChatInput({ return ctx.resourceId; } if (ctx.type === 'dashboard' && ctx.resourceId) { - const cached = queryClient.getQueryData>( - REACT_QUERY_KEY.GET_ALL_DASHBOARDS, + const cached = queryClient.getQueryData( + getListDashboardsForUserV2QueryKey(DASHBOARD_LIST_PARAMS), ); - const dash = cached?.data?.find((d) => d.id === ctx.resourceId); - if (dash?.data.title) { - return dash.data.title; + const dash = cached?.data?.dashboards?.find((d) => d.id === ctx.resourceId); + if (dash) { + return dashboardTitle(dash); } } if (ctx.type === 'alert' && ctx.resourceId) { @@ -800,9 +821,9 @@ export default function ChatInput({ const contextEntitiesByCategory: Record = { Dashboards: - dashboardsResponse?.data?.map((dashboard) => ({ + dashboardsResponse?.data?.dashboards?.map((dashboard) => ({ id: dashboard.id, - value: dashboard.data.title ?? 'Untitled', + value: dashboardTitle(dashboard), })) ?? [], Alerts: alertsResponse?.data diff --git a/frontend/src/container/AIAssistant/components/ChatInput/__tests__/ChatInput.prefill.test.tsx b/frontend/src/container/AIAssistant/components/ChatInput/__tests__/ChatInput.prefill.test.tsx index 9c0a6c7fbd6..80f0c4f2b3c 100644 --- a/frontend/src/container/AIAssistant/components/ChatInput/__tests__/ChatInput.prefill.test.tsx +++ b/frontend/src/container/AIAssistant/components/ChatInput/__tests__/ChatInput.prefill.test.tsx @@ -2,12 +2,13 @@ import { render, screen, userEvent, waitFor } from 'tests/test-utils'; // The prefill flow only depends on the context-picker data hooks resolving to // empty lists (so the empty state renders) — mock them to skip real fetches. -jest.mock('hooks/dashboard/useGetAllDashboard', () => ({ - useGetAllDashboard: (): unknown => ({ - data: [], +jest.mock('api/generated/services/dashboard', () => ({ + useListDashboardsForUserV2: (): unknown => ({ + data: undefined, isLoading: false, isError: false, }), + getListDashboardsForUserV2QueryKey: (): string[] => ['dashboards'], })); jest.mock('api/generated/services/rules', () => ({ diff --git a/frontend/src/container/AIAssistant/getAutoContexts.ts b/frontend/src/container/AIAssistant/getAutoContexts.ts index 58218254687..273e4008f70 100644 --- a/frontend/src/container/AIAssistant/getAutoContexts.ts +++ b/frontend/src/container/AIAssistant/getAutoContexts.ts @@ -2,6 +2,7 @@ import type { MessageContext } from 'api/ai-assistant/chat'; import { QueryParams } from 'constants/query'; import ROUTES from 'constants/routes'; import { AlertListTabs } from 'pages/AlertList/types'; +import { NEW_PANEL_ID } from 'pages/DashboardPageV2/DashboardContainer/PanelEditor/newPanelRoute'; import { matchPath } from 'react-router-dom'; /** @@ -30,22 +31,23 @@ export function getAutoContexts( // ── Dashboards ──────────────────────────────────────────────────────────── - // Widget edit (panel_edit) — `/dashboard/:dashboardId/:widgetId`. - const widgetMatch = matchPath<{ dashboardId: string; widgetId: string }>( + // Panel editor (V2). `panel/new` has no widget id yet and the schema requires + // a non-empty `panel_edit.widgetId`, so it reports `panel_create` instead. + const panelEditorMatch = matchPath<{ dashboardId: string; panelId: string }>( pathname, - { path: ROUTES.DASHBOARD_WIDGET, exact: true }, + { path: ROUTES.DASHBOARD_PANEL_EDITOR, exact: true }, ); - if (widgetMatch) { + if (panelEditorMatch) { + const { dashboardId, panelId } = panelEditorMatch.params; + const isNewPanel = panelId === NEW_PANEL_ID; return [ { source: 'auto', type: 'dashboard', - resourceId: widgetMatch.params.dashboardId, - metadata: { - page: 'panel_edit', - widgetId: widgetMatch.params.widgetId, - ...sharedMetadata, - }, + resourceId: dashboardId, + metadata: isNewPanel + ? { page: 'panel_create', ...sharedMetadata } + : { page: 'panel_edit', widgetId: panelId, ...sharedMetadata }, }, ]; } diff --git a/frontend/src/container/AIAssistant/resolvePageType.ts b/frontend/src/container/AIAssistant/resolvePageType.ts index 2f712621953..2f1bbbdfcd5 100644 --- a/frontend/src/container/AIAssistant/resolvePageType.ts +++ b/frontend/src/container/AIAssistant/resolvePageType.ts @@ -9,6 +9,8 @@ const PAGE_METADATA_TO_DTO: Record = { dashboard_detail: PageTypeDTO.dashboard_detail, dashboard_list: PageTypeDTO.dashboard_list, panel_edit: PageTypeDTO.panel_edit, + // There is no panel_create, so sending panel_edit temporarily + panel_create: PageTypeDTO.panel_edit, panel_fullscreen: PageTypeDTO.panel_fullscreen, logs_explorer: PageTypeDTO.logs_explorer, trace_detail: PageTypeDTO.trace_detail, From 31cb4d7520866b5e1defd6ebb166353bf1b218e5 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Mon, 27 Jul 2026 19:01:56 +0530 Subject: [PATCH 2/5] feat(logs): unwrap lone message field from json log body (#12206) --- .../v5/queryRange/convertV5Response.test.ts | 84 +++++++++++++++++++ .../api/v5/queryRange/convertV5Response.ts | 31 +++++-- 2 files changed, 110 insertions(+), 5 deletions(-) diff --git a/frontend/src/api/v5/queryRange/convertV5Response.test.ts b/frontend/src/api/v5/queryRange/convertV5Response.test.ts index c1e39aa51e1..3c7758d5bbf 100644 --- a/frontend/src/api/v5/queryRange/convertV5Response.test.ts +++ b/frontend/src/api/v5/queryRange/convertV5Response.test.ts @@ -380,4 +380,88 @@ describe('convertV5ResponseToLegacy', () => { }, }); }); + + describe('raw logs body: extract lone `message` field', () => { + function makeRawResult( + rows: Array<{ timestamp: string; data: Record }>, + type: 'raw' | 'trace' = 'raw', + ): ReturnType { + const v5Data = { + type, + data: { results: [{ queryName: 'A', rows }] }, + meta: { rowsScanned: 0, bytesScanned: 0, durationMs: 0, stepIntervals: {} }, + } as unknown as QueryRangeResponseV5; + + const params = makeBaseParams(type as RequestType, [ + { + type: 'builder_query', + spec: { + name: 'A', + signal: type === 'trace' ? 'traces' : 'logs', + stepInterval: 60, + disabled: false, + aggregations: [], + }, + }, + ]); + + const input: SuccessResponse = + makeBaseSuccess({ data: v5Data }, params); + + return convertV5ResponseToLegacy(input, { A: 'A' }, false); + } + + it('unwraps body when it is an object with only a message field', () => { + const result = makeRawResult([ + { timestamp: '2026-07-21T00:00:00Z', data: { body: { message: 'hello' } } }, + ]); + + expect(result.payload.data.result[0].list?.[0]?.data?.body).toBe('hello'); + }); + + it('leaves body unchanged when the object has keys besides message', () => { + const body = { message: 'hello', level: 'INFO' }; + const result = makeRawResult([ + { timestamp: '2026-07-21T00:00:00Z', data: { body } }, + ]); + + expect(result.payload.data.result[0].list?.[0]?.data?.body).toStrictEqual( + body, + ); + }); + + it('leaves a string body unchanged (use_json_body off)', () => { + const result = makeRawResult([ + { + timestamp: '2026-07-21T00:00:00Z', + data: { body: '{"message":"hello"}' }, + }, + ]); + + expect(result.payload.data.result[0].list?.[0]?.data?.body).toBe( + '{"message":"hello"}', + ); + }); + + it('stringifies the nested object when message is an object', () => { + const nested = { a: 1, b: 2 }; + const result = makeRawResult([ + { timestamp: '2026-07-21T00:00:00Z', data: { body: { message: nested } } }, + ]); + + expect(result.payload.data.result[0].list?.[0]?.data?.body).toBe( + JSON.stringify(nested), + ); + }); + + it('does not add a body key to rows without a body (traces)', () => { + const result = makeRawResult( + [{ timestamp: '2026-07-21T00:00:00Z', data: { name: 'span-1' } }], + 'trace', + ); + + const data = (result.payload.data.result[0].list?.[0] as any)?.data ?? {}; + expect('body' in data).toBe(false); + }); + }); }); diff --git a/frontend/src/api/v5/queryRange/convertV5Response.ts b/frontend/src/api/v5/queryRange/convertV5Response.ts index c113a71eb10..6e9bd73deac 100644 --- a/frontend/src/api/v5/queryRange/convertV5Response.ts +++ b/frontend/src/api/v5/queryRange/convertV5Response.ts @@ -273,6 +273,19 @@ function convertScalarWithFormatForWeb( }); } +function extractOnlyMessageBody(body: unknown): unknown { + const isJsonBody = body && typeof body === 'object' && !Array.isArray(body); + if (isJsonBody) { + const keys = Object.keys(body); + const hasOnlyMessageKey = keys.length === 1 && keys[0] === 'message'; + if (hasOnlyMessageKey) { + const { message } = body as { message: unknown }; + return typeof message === 'string' ? message : JSON.stringify(message); + } + } + return body; +} + /** * Converts V5 RawData to legacy format */ @@ -285,14 +298,22 @@ function convertRawData( queryName: rawData.queryName, legend: legendMap[rawData.queryName] || rawData.queryName, series: null, - list: rawData.rows?.map((row) => ({ - timestamp: row.timestamp, - data: { + list: rawData.rows?.map((row) => { + const data = { // Map raw data to ILog structure - spread row.data first to include all properties ...row.data, date: row.timestamp, - } as any, - })), + } as any; + + if ('body' in row.data) { + data.body = extractOnlyMessageBody(row.data.body); + } + + return { + timestamp: row.timestamp, + data, + }; + }), nextCursor: rawData.nextCursor, }; } From 38639847be06997780c19a139851fe83afc9c203 Mon Sep 17 00:00:00 2001 From: Naman Verma Date: Mon, 27 Jul 2026 21:18:18 +0530 Subject: [PATCH 3/5] fix: clamp over-wide widgets, drop zero-width widgets, and properly handle invalid order by (#12295) --- .../dashboardtypes/perses_v1_to_v2_layouts.go | 8 ++ .../perses_v1_to_v2_queries_malformed.go | 100 ++++++++++++++---- .../dashboardtypes/perses_v1_to_v2_test.go | 88 +++++++++++++++ 3 files changed, 175 insertions(+), 21 deletions(-) diff --git a/pkg/types/dashboardtypes/perses_v1_to_v2_layouts.go b/pkg/types/dashboardtypes/perses_v1_to_v2_layouts.go index 13fa2cee6a9..c9ee4d5490e 100644 --- a/pkg/types/dashboardtypes/perses_v1_to_v2_layouts.go +++ b/pkg/types/dashboardtypes/perses_v1_to_v2_layouts.go @@ -180,6 +180,11 @@ func placedWidgetIDs(data StorableDashboardData) map[string]bool { for _, e := range layout { if m, ok := e.(map[string]any); ok { if i, ok := m["i"].(string); ok && i != "" { + // A zero-width placement doesn't render in the v1 UI; treat it as + // unplaced so the widget is dropped rather than migrated invisibly. + if w, ok := m["w"].(float64); ok && w <= 0 { + continue + } ids[i] = true } } @@ -320,6 +325,9 @@ func compactGridItemsVertically(items []dashboard.GridItem) { if l.X < 0 { l.X = 0 } + if l.X+l.Width > gridColumnCount { // still overflows (wider than the grid) → clamp width so x+width = cols + l.Width = gridColumnCount - l.X + } if l.Y < 0 { l.Y = 0 } diff --git a/pkg/types/dashboardtypes/perses_v1_to_v2_queries_malformed.go b/pkg/types/dashboardtypes/perses_v1_to_v2_queries_malformed.go index ebb56401f8e..1d3369f1f6f 100644 --- a/pkg/types/dashboardtypes/perses_v1_to_v2_queries_malformed.go +++ b/pkg/types/dashboardtypes/perses_v1_to_v2_queries_malformed.go @@ -3,6 +3,7 @@ package dashboardtypes import ( "context" "encoding/json" + "fmt" "log/slog" "regexp" "strings" @@ -108,26 +109,12 @@ func normalizeFunctionArgs(query map[string]any) { } } -// malformedOrderByValueKeys are v4 order-by columnNames meaning "order by the aggregation value" -// that the v5 aggregation validator rejects (validateOrderByForAggregation). All resolve -// to the same aggregation key. Add more as they surface. The frontend passes these -// through (the query-service resolves them), but the v2 dashboard validator only accepts -// a real aggregation key. -var malformedOrderByValueKeys = map[string]bool{ - "#SIGNOZ_VALUE": true, - "A": true, - "A.count()": true, - "__result": true, - "value": true, - "A.p99(duration_nano)": true, - "aws_Kafka_MessagesInPerSec_max": true, - "byte_in_count": true, - "(http_server_request_duration_ms.bucket)": true, -} - -// normalizeOrderByKeys rewrites any orderBy columnName in orderByValueKeys to the -// v5-valid aggregation key. Left untouched if the key can't resolve (no aggregation to -// name). +// normalizeOrderByKeys rewrites any orderBy columnName the v5 aggregation validator +// would reject (validateOrderByForAggregation) to the canonical aggregation value key. +// v1 tolerated free-form "order by the value" aliases (#SIGNOZ_VALUE, the query name, +// the raw metric/expression) that the query-service resolved at query time; v2 accepts +// only a real order key. Anything already valid (a group-by key, an aggregation +// expression/alias/index) is left alone. No-op if no aggregation key can be named. func normalizeOrderByKeys(query map[string]any) { orders, ok := query["orderBy"].([]any) if !ok { @@ -137,17 +124,88 @@ func normalizeOrderByKeys(query map[string]any) { if !ok { return } + valid := validAggregationOrderKeys(query) for _, o := range orders { order, ok := o.(map[string]any) if !ok { continue } - if cn, _ := order["columnName"].(string); malformedOrderByValueKeys[cn] { + if cn, _ := order["columnName"].(string); cn != "" && !valid[cn] { order["columnName"] = key } } } +// validAggregationOrderKeys is a line-for-line mirror of validateOrderByForAggregation +// (querybuildertypesv5/validation.go) over the untyped query map instead of a typed +// QueryBuilderQuery. Keep the two in lockstep — every insertion here must match one +// there — so a side-by-side read makes any drift obvious. The only adaptations: fields +// are read out of maps, the type switch on the aggregation becomes a switch on the +// query signal (metrics vs logs/traces), and a not-yet-upgraded group-by still carries +// the v4 "key" instead of the v5 "name". +func validAggregationOrderKeys(query map[string]any) map[string]bool { + validOrderKeys := make(map[string]bool) + + for _, gb := range asObjects(query["groupBy"]) { + name, _ := gb["name"].(string) + if name == "" { + name, _ = gb["key"].(string) + } + validOrderKeys[name] = true + } + + signal := signalFromDataSource(query["dataSource"]) + for i, agg := range asObjects(query["aggregations"]) { + validOrderKeys[fmt.Sprintf("%d", i)] = true + + switch signal { + // TraceAggregation / LogAggregation (identical bodies in the validator). + case telemetrytypes.SignalTraces, telemetrytypes.SignalLogs: + if alias, _ := agg["alias"].(string); alias != "" { + validOrderKeys[alias] = true + } + expression, _ := agg["expression"].(string) + validOrderKeys[expression] = true + + // MetricAggregation. + case telemetrytypes.SignalMetrics: + // Also allow the generic __result pattern + validOrderKeys["__result"] = true + + metricName, _ := agg["metricName"].(string) + spaceRaw, _ := agg["spaceAggregation"].(string) + timeRaw, _ := agg["timeAggregation"].(string) + spaceAggregation := metrictypes.SpaceAggregation{String: valuer.NewString(spaceRaw)} + timeAggregation := metrictypes.TimeAggregation{String: valuer.NewString(timeRaw)} + + validOrderKeys[fmt.Sprintf("%s(%s)", spaceAggregation.StringValue(), metricName)] = true + if timeAggregation != metrictypes.TimeAggregationUnspecified { + validOrderKeys[fmt.Sprintf("%s(%s)", timeAggregation.StringValue(), metricName)] = true + } + if timeAggregation != metrictypes.TimeAggregationUnspecified && spaceAggregation != metrictypes.SpaceAggregationUnspecified { + validOrderKeys[fmt.Sprintf("%s(%s(%s))", spaceAggregation.StringValue(), timeAggregation.StringValue(), metricName)] = true + } + } + } + + return validOrderKeys +} + +// asObjects returns the map elements of a []any, skipping non-object entries. +func asObjects(raw any) []map[string]any { + items, ok := raw.([]any) + if !ok { + return nil + } + out := make([]map[string]any, 0, len(items)) + for _, it := range items { + if m, ok := it.(map[string]any); ok { + out = append(out, m) + } + } + return out +} + // aggregationOrderKey names the first aggregation the way validateOrderByForAggregation // expects: "space(metricName)" for metrics, the expression for logs/traces. func aggregationOrderKey(query map[string]any) (string, bool) { diff --git a/pkg/types/dashboardtypes/perses_v1_to_v2_test.go b/pkg/types/dashboardtypes/perses_v1_to_v2_test.go index 5ff48273851..a84348a65b9 100644 --- a/pkg/types/dashboardtypes/perses_v1_to_v2_test.go +++ b/pkg/types/dashboardtypes/perses_v1_to_v2_test.go @@ -1738,6 +1738,47 @@ func TestConvertV1WidgetQueryRewritesValueOrderKeyAfterDefaultAggregation(t *tes assert.Equal(t, "count()", spec.Order[0].Key.Name, "#SIGNOZ_VALUE resolves to the injected default aggregation") } +// A metric query ordering by a value alias the v5 validator rejects (here the raw +// metric name, never enumerated anywhere) is rewritten to the canonical aggregation +// key, while a genuine group-by order key is left alone. Guards the allowlist +// approach: validity is derived from the query, not a hardcoded set of bad keys. +func TestConvertV1WidgetQueryRewritesUnknownMetricValueOrderKey(t *testing.T) { + widget := map[string]any{ + "id": "m-1", + "panelTypes": "graph", + "query": map[string]any{ + "queryType": "builder", + "builder": map[string]any{ + "queryData": []any{ + map[string]any{ + "queryName": "A", + "expression": "A", + "dataSource": "metrics", + "aggregations": []any{map[string]any{"metricName": "http_requests_total", "spaceAggregation": "sum"}}, + "groupBy": []any{map[string]any{"key": "service.name", "dataType": "string", "type": "resource"}}, + "orderBy": []any{ + map[string]any{"columnName": "http_requests_total", "order": "desc"}, + map[string]any{"columnName": "service.name", "order": "asc"}, + }, + }, + }, + }, + }, + } + + queries := (&v1Decoder{}).convertV1WidgetQuery(widget, PanelKindTimeSeries) + require.Len(t, queries, 1) + + wrapper, ok := queries[0].Spec.Plugin.Spec.(*BuilderQuerySpec) + require.True(t, ok) + spec, ok := wrapper.Spec.(qb.QueryBuilderQuery[qb.MetricAggregation]) + require.True(t, ok, "metrics query should dispatch to MetricAggregation, got %T", wrapper.Spec) + + require.Len(t, spec.Order, 2) + assert.Equal(t, "sum(http_requests_total)", spec.Order[0].Key.Name, "unknown value alias rewritten to the aggregation key") + assert.Equal(t, "service.name", spec.Order[1].Key.Name, "valid group-by order key left alone") +} + func TestConvertV1WidgetQueryInjectsCountForNoopOnAggregationPanel(t *testing.T) { // A logs query with the list-style "noop" operator placed on an aggregation // panel (graph). createAggregationsShapeSafe drops noop, leaving no aggregation; @@ -2084,6 +2125,26 @@ func TestConvertV1LayoutsClampsXBounds(t *testing.T) { assert.Equal(t, 6, grid.Items[1].X) // x+w=16>12 shifted left to 12-6 } +func TestConvertV1LayoutsClampsOverwideWidth(t *testing.T) { + // A widget wider than the 12-col grid (e.g. carried over from a 24-col v1 grid) + // can't be shifted to fit, so its width is clamped so x+width = grid width — + // otherwise it overflows and v2 validation rejects it. + data := StorableDashboardData{ + "widgets": []any{map[string]any{"id": "wide", "panelTypes": "graph", "query": singleLogsBuilderQuery()}}, + "layout": []any{map[string]any{"i": "wide", "x": float64(0), "y": float64(0), "w": float64(24), "h": float64(6)}}, + } + + d := &v1Decoder{} + layouts := d.convertV1Layouts(data, d.convertV1Panels(data["widgets"])) + require.NoError(t, d.errIfHasMalformedFields()) + require.Len(t, layouts, 1) + grid, ok := layouts[0].Spec.(*dashboard.GridLayoutSpec) + require.True(t, ok) + require.Len(t, grid.Items, 1) + assert.Equal(t, 0, grid.Items[0].X) + assert.Equal(t, 12, grid.Items[0].Width) // w=24 clamped to 12 so x+width = 12 +} + // TestConvertV1LayoutsToleratesNonObjectPanelMap covers templates that store // panelMap as {rowID: []widgetID} instead of the canonical {rowID: {widgets, // collapsed}}. The frontend reads such an entry as "not collapsed" (it accesses @@ -2152,6 +2213,33 @@ func TestConvertV1LayoutsDropsEntryForUnrenderableWidget(t *testing.T) { assert.Equal(t, "#/spec/panels/p-1", spec.Items[0].Content.Ref) } +func TestRetainPlacedWidgetsDropsZeroWidth(t *testing.T) { + // z-1 is placed with zero width, which the v1 UI doesn't render. It must be + // dropped entirely: no panel, and no dangling layout entry. + data := StorableDashboardData{ + "widgets": []any{ + map[string]any{"id": "p-1", "panelTypes": "graph", "query": singleLogsBuilderQuery()}, + map[string]any{"id": "z-1", "panelTypes": "graph", "query": singleLogsBuilderQuery()}, + }, + "layout": []any{ + map[string]any{"i": "p-1", "x": float64(0), "y": float64(0), "w": float64(6), "h": float64(6)}, + map[string]any{"i": "z-1", "x": float64(6), "y": float64(0), "w": float64(0), "h": float64(6)}, + }, + } + + d := &v1Decoder{} + panels := d.convertV1Panels(retainPlacedWidgets(data)) + require.Contains(t, panels, "p-1") + require.NotContains(t, panels, "z-1", "a zero-width widget is not retained → no panel") + + layouts := d.convertV1Layouts(data, panels) + require.Len(t, layouts, 1) + spec, ok := layouts[0].Spec.(*dashboard.GridLayoutSpec) + require.True(t, ok) + require.Len(t, spec.Items, 1, "the zero-width widget's layout entry is dropped too") + assert.Equal(t, "#/spec/panels/p-1", spec.Items[0].Content.Ref) +} + func TestConvertV1LayoutsDropsCollapsedChildWithNoPanel(t *testing.T) { // A collapsed section lists a child ("ghost") that has no widget at all — a // deleted widget still referenced in panelMap. It produces no panel and no From 1255637e25a0342a6511b9b58eca5152a7f4a15c Mon Sep 17 00:00:00 2001 From: Vikrant Gupta Date: Mon, 27 Jul 2026 21:22:06 +0530 Subject: [PATCH 4/5] fix(authz): changelog column type for postgres metastore (#12299) --- pkg/sqlmigration/101_add_telemetry_tuples.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sqlmigration/101_add_telemetry_tuples.go b/pkg/sqlmigration/101_add_telemetry_tuples.go index c13d371bf8f..030359299b3 100644 --- a/pkg/sqlmigration/101_add_telemetry_tuples.go +++ b/pkg/sqlmigration/101_add_telemetry_tuples.go @@ -103,7 +103,7 @@ func (migration *addTelemetryTuples) Up(ctx context.Context, db *bun.DB) error { INSERT INTO changelog (store, object_type, object_id, relation, _user, operation, ulid, inserted_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (store, ulid, object_type) DO NOTHING`, - storeID, tuple.objectType, objectID, tuple.relation, user, "TUPLE_OPERATION_WRITE", tupleID, now, + storeID, tuple.objectType, objectID, tuple.relation, user, 0, tupleID, now, ) if err != nil { return err From 1483fbd2c5edda26d382956a909e42ed3a227a98 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Mon, 27 Jul 2026 21:30:29 +0530 Subject: [PATCH 5/5] fix(query-builder): coerce table sort values to string to avoid localeCompare crash (#12289) The QueryTable column sorter called `.localeCompare` on values cast with `as string`, which is compile-time only. Upstream `processTableRowValue` normalizes numeric-looking strings to real numbers while null becomes the string "N/A", so a grouped column can mix numbers and text. Comparing a number cell against a text cell skipped the numeric branch and invoked `(200).localeCompare(...)`, throwing "localeCompare is not a function" and crashing the table on sort. Extract the compare logic into `compareTableColumnValues` and coerce both sides with `String(x ?? '')` so any value type sorts safely while preserving empty-string semantics for null/undefined. Affects all QueryTable surfaces: Traces/Logs table views, APM Top Operations, and dashboard Table panels. Fixes SIGNOZ-UI-5GC --- .../createTableColumnsFromQuery.test.ts | 94 +++++++++++++++++++ .../lib/query/createTableColumnsFromQuery.ts | 29 +++--- 2 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 frontend/src/lib/query/__tests__/createTableColumnsFromQuery.test.ts diff --git a/frontend/src/lib/query/__tests__/createTableColumnsFromQuery.test.ts b/frontend/src/lib/query/__tests__/createTableColumnsFromQuery.test.ts new file mode 100644 index 00000000000..736acf13346 --- /dev/null +++ b/frontend/src/lib/query/__tests__/createTableColumnsFromQuery.test.ts @@ -0,0 +1,94 @@ +import { + compareTableColumnValues, + RowData, +} from '../createTableColumnsFromQuery'; + +// Builds a minimal RowData row. Values are intentionally loosely typed because +// real query responses can put objects/arrays into cells despite RowData's +// declared `string | number` index signature (that mismatch is the bug under test). +const row = (value: unknown, dataIndex = 'col'): RowData => + ({ + timestamp: 0, + key: 'k', + [dataIndex]: value, + }) as unknown as RowData; + +describe('compareTableColumnValues', () => { + it('sorts numerically when both cells are numbers', () => { + expect(compareTableColumnValues(row(1), row(2), 'col')).toBeLessThan(0); + expect(compareTableColumnValues(row(2), row(1), 'col')).toBeGreaterThan(0); + expect(compareTableColumnValues(row(5), row(5), 'col')).toBe(0); + }); + + it('sorts numeric-looking strings numerically, not lexically', () => { + // "10" vs "9": numeric branch => 10 - 9 > 0. A string compare would be < 0. + expect(compareTableColumnValues(row('10'), row('9'), 'col')).toBeGreaterThan( + 0, + ); + }); + + it('prefers the `_without_unit` value for numeric comparison', () => { + const a = row('2 ms'); + const b = row('10 ms'); + a.col_without_unit = 2; + b.col_without_unit = 10; + expect(compareTableColumnValues(a, b, 'col')).toBeLessThan(0); + }); + + it('falls back to locale string compare for non-numeric strings', () => { + expect(compareTableColumnValues(row('abc'), row('abd'), 'col')).toBe( + 'abc'.localeCompare('abd'), + ); + }); + + it('treats null/undefined cells as empty string (no "null"/"undefined")', () => { + // Empty string sorts before a real word, and two empties are equal. + expect(compareTableColumnValues(row(null), row('a'), 'col')).toBeLessThan(0); + expect(compareTableColumnValues(row(undefined), row(undefined), 'col')).toBe( + 0, + ); + // If null coerced to the literal "null", this would sort after "a". + expect( + compareTableColumnValues(row(null), row('a'), 'col'), + ).not.toBeGreaterThan(0); + }); + + it('does not throw when a cell value is an object', () => { + const a = row({ foo: 'bar' }); + const b = row({ foo: 'baz' }); + expect(() => compareTableColumnValues(a, b, 'col')).not.toThrow(); + expect(typeof compareTableColumnValues(a, b, 'col')).toBe('number'); + }); + + it('does not throw when a cell value is an array', () => { + const a = row([1, 2]); + const b = row([3, 4]); + expect(() => compareTableColumnValues(a, b, 'col')).not.toThrow(); + // String([1,2]) === "1,2" < String([3,4]) === "3,4" + expect(compareTableColumnValues(a, b, 'col')).toBeLessThan(0); + }); + + it('does not throw when one cell is numeric and the other is an object', () => { + const a = row(42); + const b = row({ foo: 'bar' }); + expect(() => compareTableColumnValues(a, b, 'col')).not.toThrow(); + expect(typeof compareTableColumnValues(a, b, 'col')).toBe('number'); + }); + + it('does not throw for a number cell compared against an "N/A" cell', () => { + // http.status_code column: [null, "200", ...] -> ["N/A", 200, ...] + const numberCell = row(200); // numeric string "200" becomes the number 200 + const naCell = row('N/A'); // null becomes the string "N/A" + + // Both orderings — antd's sorter compares pairs in both directions. + expect(() => + compareTableColumnValues(numberCell, naCell, 'col'), + ).not.toThrow(); + expect(() => + compareTableColumnValues(naCell, numberCell, 'col'), + ).not.toThrow(); + expect(typeof compareTableColumnValues(numberCell, naCell, 'col')).toBe( + 'number', + ); + }); +}); diff --git a/frontend/src/lib/query/createTableColumnsFromQuery.ts b/frontend/src/lib/query/createTableColumnsFromQuery.ts index fd5e14f4d8f..a5e876c1b50 100644 --- a/frontend/src/lib/query/createTableColumnsFromQuery.ts +++ b/frontend/src/lib/query/createTableColumnsFromQuery.ts @@ -637,6 +637,21 @@ const generateData = ( return data; }; +export const compareTableColumnValues = ( + a: RowData, + b: RowData, + dataIndex: string, +): number => { + const valueA = Number(a[`${dataIndex}_without_unit`] ?? a[dataIndex]); + const valueB = Number(b[`${dataIndex}_without_unit`] ?? b[dataIndex]); + + if (!isNaN(valueA) && !isNaN(valueB)) { + return valueA - valueB; + } + + return String(a[dataIndex] ?? '').localeCompare(String(b[dataIndex] ?? '')); +}; + const generateTableColumns = ( dynamicColumns: DynamicColumns, renderColumnCell?: QueryTableProps['renderColumnCell'], @@ -650,18 +665,8 @@ const generateTableColumns = ( title: item.title, width: QUERY_TABLE_CONFIG.width, render: renderColumnCell && renderColumnCell[dataIndex], - sorter: (a: RowData, b: RowData): number => { - const valueA = Number(a[`${dataIndex}_without_unit`] ?? a[dataIndex]); - const valueB = Number(b[`${dataIndex}_without_unit`] ?? b[dataIndex]); - - if (!isNaN(valueA) && !isNaN(valueB)) { - return valueA - valueB; - } - - return ((a[dataIndex] as string) || '').localeCompare( - (b[dataIndex] as string) || '', - ); - }, + sorter: (a: RowData, b: RowData): number => + compareTableColumnValues(a, b, dataIndex), }; return [...acc, column];