From bd69fc2c1c1b8bcf68c694b4efb9f7b2b337d58d Mon Sep 17 00:00:00 2001 From: espenblyn Date: Tue, 18 Aug 2026 19:04:28 +0200 Subject: [PATCH 1/4] fix preserve color rules for SQL and PromQL charts --- .changeset/large-suits-smoke.md | 5 ++ .../ChartEditor/__tests__/utils.test.ts | 46 +++++++++++++++++++ .../app/src/components/ChartEditor/utils.ts | 4 ++ 3 files changed, 55 insertions(+) create mode 100644 .changeset/large-suits-smoke.md diff --git a/.changeset/large-suits-smoke.md b/.changeset/large-suits-smoke.md new file mode 100644 index 0000000000..182fde5e03 --- /dev/null +++ b/.changeset/large-suits-smoke.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/app": patch +--- + +Preserve conditional color rules for SQL and PromQl chart configs diff --git a/packages/app/src/components/ChartEditor/__tests__/utils.test.ts b/packages/app/src/components/ChartEditor/__tests__/utils.test.ts index aed30696f9..2d17a383a3 100644 --- a/packages/app/src/components/ChartEditor/__tests__/utils.test.ts +++ b/packages/app/src/components/ChartEditor/__tests__/utils.test.ts @@ -1575,6 +1575,52 @@ describe('color round-trip (sql/promql Number tile)', () => { expect((result as any).color).toBe('chart-error'); }); + it('preserves colorRules through both conversions for sql Number tile', () => { + const form: ChartEditorFormState = { + configType: 'sql', + displayType: DisplayType.Number, + sqlTemplate: 'SELECT count() FROM logs', + connection: 'conn-1', + colorRules: [ + { operator: 'lt', value: 4, color: 'chart-error' }, + { operator: 'gte', value: 4, color: 'chart-success' }, + ], + series: [], + }; + + expect(convertFormStateToSavedChartConfig(form, undefined)).toMatchObject({ + colorRules: form.colorRules, + }); + expect( + convertFormStateToChartConfig(form, dateRange, undefined), + ).toMatchObject({ + colorRules: form.colorRules, + }); + }); + + it('preserves colorRules through both conversions for promql Number tile', () => { + const form: ChartEditorFormState = { + configType: 'promql', + displayType: DisplayType.Number, + promqlExpression: 'up', + connection: 'conn-1', + colorRules: [ + { operator: 'lt', value: 1, color: 'chart-error' }, + { operator: 'gte', value: 1, color: 'chart-success' }, + ], + series: [], + }; + + expect(convertFormStateToSavedChartConfig(form, undefined)).toMatchObject({ + colorRules: form.colorRules, + }); + expect( + convertFormStateToChartConfig(form, dateRange, undefined), + ).toMatchObject({ + colorRules: form.colorRules, + }); + }); + it('omits color when not set on sql Number tile', () => { const form: ChartEditorFormState = { configType: 'sql', diff --git a/packages/app/src/components/ChartEditor/utils.ts b/packages/app/src/components/ChartEditor/utils.ts index 830f6c1865..bcf9449a7d 100644 --- a/packages/app/src/components/ChartEditor/utils.ts +++ b/packages/app/src/components/ChartEditor/utils.ts @@ -157,6 +157,7 @@ export function convertFormStateToSavedChartConfig( 'displayType', 'numberFormat', 'color', + 'colorRules', 'granularity', 'compareToPreviousPeriod', 'fillNulls', @@ -180,6 +181,7 @@ export function convertFormStateToSavedChartConfig( 'displayType', 'numberFormat', 'color', + 'colorRules', 'granularity', 'compareToPreviousPeriod', 'fillNulls', @@ -238,6 +240,7 @@ export function convertFormStateToChartConfig( 'displayType', 'numberFormat', 'color', + 'colorRules', 'granularity', 'compareToPreviousPeriod', 'fillNulls', @@ -261,6 +264,7 @@ export function convertFormStateToChartConfig( 'displayType', 'numberFormat', 'color', + 'colorRules', 'granularity', 'compareToPreviousPeriod', 'fillNulls', From f406edef324caca39d804e926a3f6b1c16c0ece3 Mon Sep 17 00:00:00 2001 From: espenblyn Date: Tue, 18 Aug 2026 19:40:12 +0200 Subject: [PATCH 2/4] fix raw SQL color rules in external dashboard API --- packages/api/openapi.json | 10 ++++- .../__tests__/dashboards.int.test.ts | 39 +++++++++++++++++-- .../src/routers/external-api/v2/dashboards.ts | 11 +++++- .../external-api/v2/utils/dashboards.ts | 10 ++--- packages/api/src/utils/zod.ts | 14 ++----- 5 files changed, 62 insertions(+), 22 deletions(-) diff --git a/packages/api/openapi.json b/packages/api/openapi.json index 341c19fba7..2507f93e5a 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -2302,7 +2302,15 @@ }, "color": { "$ref": "#/components/schemas/ChartPaletteToken", - "description": "Optional static color applied to the displayed number. Raw SQL number tiles do not support conditional colorRules.\n" + "description": "Optional static color applied to the displayed number." + }, + "colorRules": { + "type": "array", + "maxItems": 10, + "description": "Ordered conditional color rules evaluated against the displayed value (last match wins). Falls back to color, then the default text color when no rule matches.\n", + "items": { + "$ref": "#/components/schemas/NumberTileColorCondition" + } } } } diff --git a/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts b/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts index 3b97264360..23d101a9c1 100644 --- a/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts @@ -5455,21 +5455,52 @@ describe('External API v2 Dashboards - new format', () => { }); }); - it('strips colorRules from a raw SQL number tile, keeping color', async () => { + it('round-trips colorRules for a raw SQL number tile', async () => { + const colorRules = [ + { operator: 'gt', value: 1, color: 'chart-red' }, + { operator: 'lte', value: 1, color: 'chart-green' }, + ]; + const create = await authRequest('post', BASE_URL) .send({ name: 'Raw SQL colorRules', tiles: [ rawSqlNumberTile({ color: 'chart-blue', - colorRules: [{ operator: 'gt', value: 1, color: 'chart-red' }], + colorRules, }), ], tags: [], }) .expect(200); - expect(create.body.data.tiles[0].config.color).toBe('chart-blue'); - expect(create.body.data.tiles[0].config.colorRules).toBeUndefined(); + + expect(create.body.data.tiles[0].config).toMatchObject({ + color: 'chart-blue', + colorRules, + }); + + const dashboardId = create.body.data.id; + const get = await authRequest('get', `${BASE_URL}/${dashboardId}`).expect( + 200, + ); + + expect(get.body.data.tiles[0].config).toMatchObject({ + color: 'chart-blue', + colorRules, + }); + + const update = await authRequest('put', `${BASE_URL}/${dashboardId}`) + .send({ + name: get.body.data.name, + tiles: get.body.data.tiles, + tags: get.body.data.tags, + }) + .expect(200); + + expect(update.body.data.tiles[0].config).toMatchObject({ + color: 'chart-blue', + colorRules, + }); }); // ── Negative: one per schema rejection rule ───────────────────────── diff --git a/packages/api/src/routers/external-api/v2/dashboards.ts b/packages/api/src/routers/external-api/v2/dashboards.ts index 5e868f85ca..fb175693df 100644 --- a/packages/api/src/routers/external-api/v2/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/dashboards.ts @@ -1174,9 +1174,16 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * example: "number" * color: * $ref: '#/components/schemas/ChartPaletteToken' + * description: Optional static color applied to the displayed number. + * colorRules: + * type: array + * maxItems: 10 * description: > - * Optional static color applied to the displayed number. Raw - * SQL number tiles do not support conditional colorRules. + * Ordered conditional color rules evaluated against the displayed + * value (last match wins). Falls back to color, then the default + * text color when no rule matches. + * items: + * $ref: '#/components/schemas/NumberTileColorCondition' * * PieRawSqlChartConfig: * description: Raw SQL configuration for a pie chart. diff --git a/packages/api/src/routers/external-api/v2/utils/dashboards.ts b/packages/api/src/routers/external-api/v2/utils/dashboards.ts index 6700532138..0421e58aeb 100644 --- a/packages/api/src/routers/external-api/v2/utils/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/utils/dashboards.ts @@ -237,10 +237,8 @@ const convertToExternalTileChartConfig = ( sqlTemplate: config.sqlTemplate, sourceId: config.source, numberFormat: config.numberFormat, - // Raw SQL number tiles carry the static tile color too (no - // colorRules; see the schema). Normalize a legacy token saved - // before the hue rename to its hue name on output. color: resolveChartPaletteToken(config.color), + colorRules: toExternalColorRules(config.colorRules), }; case DisplayType.Pie: return { @@ -685,12 +683,14 @@ export function convertToInternalTileConfig( externalConfig.displayType === 'table' ? externalConfig.onClick : undefined, - // Only the raw SQL number variant carries `color`; table and pie - // do not expose it. `_.omitBy(_.isNil)` below drops it when absent. color: externalConfig.displayType === 'number' ? externalConfig.color : undefined, + colorRules: + externalConfig.displayType === 'number' + ? externalConfig.colorRules + : undefined, } satisfies RawSqlSavedChartConfig; break; default: diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 4dd85762c9..c8c011200c 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -321,14 +321,8 @@ const externalDashboardTableRawSqlChartConfigSchema = const externalDashboardNumberRawSqlChartConfigSchema = externalDashboardRawSqlChartConfigBaseSchema.extend({ displayType: z.literal('number'), - // Raw SQL number tiles expose the same static tile color as builder - // number tiles: the editor gates the picker on displayType, not - // configType (`ChartDisplaySettingsDrawer`). `colorRules` is - // intentionally omitted here because the editor's save path - // (`convertFormStateToSavedChartConfig`) picks `color` but not - // `colorRules` for raw SQL configs, so persisted raw SQL number tiles - // never carry rules. color: ChartPaletteTokenSchema.optional(), + colorRules: z.array(NumberTileColorConditionSchema).max(10).optional(), }); const externalDashboardPieRawSqlChartConfigSchema = @@ -368,9 +362,9 @@ const externalDashboardNumberChartConfigSchema = z.object({ // `configType === 'sql'`). The save path // (`convertFormStateToSavedChartConfig`) persists `backgroundChart` only on // the builder branch (the raw SQL / promql picks omit it), so it lives on - // the builder number schema only, like `colorRules`. `BackgroundChartSchema` - // is imported from common-utils so the external surface cannot drift from - // what the UI persists. + // the builder number schema only. `BackgroundChartSchema` is imported from + // common-utils so the external surface cannot drift from what the UI + // persists. backgroundChart: BackgroundChartSchema.optional(), }); From 2df59d186b6ffe315201b2e92efb80f3d4b771ce Mon Sep 17 00:00:00 2001 From: espenblyn Date: Tue, 18 Aug 2026 23:33:15 +0200 Subject: [PATCH 3/4] test raw SQL color rules --- .../__tests__/dashboards.int.test.ts | 75 ++++++++++++++++--- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts b/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts index 23d101a9c1..67d93b3357 100644 --- a/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts @@ -3073,7 +3073,7 @@ describe('External API v2 Dashboards - new format', () => { sqlTemplate, sourceId, numberFormat: { output: 'currency', currencySymbol: '$' }, - // Raw SQL number tiles carry the static tile color (no colorRules). + // This fixture exercises the static color; raw SQL number tiles also support colorRules. color: 'chart-purple', }, }; @@ -4783,7 +4783,7 @@ describe('External API v2 Dashboards - new format', () => { sqlTemplate, sourceId, numberFormat: { output: 'currency', currencySymbol: '$' }, - // Raw SQL number tiles carry the static tile color (no colorRules). + // This fixture exercises the static color; raw SQL number tiles also support colorRules. color: 'chart-purple', }, }; @@ -5360,6 +5360,13 @@ describe('External API v2 Dashboards - new format', () => { }, }); + const postRawSqlTile = (config: Record) => + authRequest('post', BASE_URL).send({ + name: 'Raw SQL number color dashboard', + tiles: [rawSqlNumberTile(config)], + tags: [], + }); + // ── Positive: one per UI input ────────────────────────────────────── it('round-trips a builder number tile with a static color', async () => { @@ -5530,6 +5537,16 @@ describe('External API v2 Dashboards - new format', () => { expect(res.body.message).toContain('tiles.0.config.colorRules'); }); + it('rejects more than 10 colorRules for a raw SQL number tile', async () => { + const colorRules = Array.from({ length: 11 }, (_, i) => ({ + operator: 'gt', + value: i, + color: 'chart-blue', + })); + const res = await postRawSqlTile({ colorRules }).expect(400); + expect(res.body.message).toContain('tiles.0.config.colorRules'); + }); + it('rejects a between rule whose value is not a two-number tuple', async () => { await postTile({ colorRules: [{ operator: 'between', value: 100, color: 'chart-blue' }], @@ -5551,6 +5568,15 @@ describe('External API v2 Dashboards - new format', () => { } }); + it('rejects unsupported colorRule operators for a raw SQL number tile', async () => { + for (const operator of ['contains', 'startsWith', 'endsWith', 'regex']) { + const res = await postRawSqlTile({ + colorRules: [{ operator, value: 'error', color: 'chart-blue' }], + }).expect(400); + expect(res.body.message).toContain('tiles.0.config.colorRules'); + } + }); + it('rejects a per-rule color that is not a palette token', async () => { const res = await postTile({ colorRules: [{ operator: 'gt', value: 1, color: 'red' }], @@ -5562,6 +5588,17 @@ describe('External API v2 Dashboards - new format', () => { }).expect(400); }); + it('rejects a per-rule color that is not a palette token for a raw SQL number tile', async () => { + const res = await postRawSqlTile({ + colorRules: [{ operator: 'gt', value: 1, color: 'red' }], + }).expect(400); + expect(res.body.message).toContain('tiles.0.config.colorRules'); + + await postRawSqlTile({ + colorRules: [{ operator: 'gt', value: 1, color: 'chart-1' }], + }).expect(400); + }); + it('rejects a rule label longer than 40 characters', async () => { await postTile({ colorRules: [ @@ -5660,13 +5697,7 @@ describe('External API v2 Dashboards - new format', () => { }); it('normalizes a legacy numeric token on a raw SQL number tile to its hue name on read', async () => { - const create = await authRequest('post', BASE_URL) - .send({ - name: 'Raw SQL legacy color', - tiles: [rawSqlNumberTile({ color: 'chart-blue' })], - tags: [], - }) - .expect(200); + const create = await postRawSqlTile({ color: 'chart-blue' }).expect(200); const dashboardId = create.body.data.id; await Dashboard.updateOne( @@ -5680,6 +5711,32 @@ describe('External API v2 Dashboards - new format', () => { // chart-4 maps to chart-red. expect(get.body.data.tiles[0].config.color).toBe('chart-red'); }); + + it('normalizes legacy raw SQL colorRule colors and drops unresolvable ones on read', async () => { + const create = await postRawSqlTile({ + colorRules: [{ operator: 'gt', value: 1, color: 'chart-green' }], + }).expect(200); + const dashboardId = create.body.data.id; + + await Dashboard.updateOne( + { _id: dashboardId }, + { + $set: { + 'tiles.0.config.colorRules': [ + { operator: 'gt', value: 1, color: 'chart-1' }, + { operator: 'gt', value: 2, color: 'not-a-token' }, + ], + }, + }, + ); + + const get = await authRequest('get', `${BASE_URL}/${dashboardId}`).expect( + 200, + ); + expect(get.body.data.tiles[0].config.colorRules).toEqual([ + { operator: 'gt', value: 1, color: 'chart-green' }, + ]); + }); }); describe('Number tile background chart (HDX-1360)', () => { From d929d56b4893471c332d3decb5399f548a97081d Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Thu, 3 Sep 2026 15:11:53 -0400 Subject: [PATCH 4/4] fix: Add colorRules to MCP raw sql tile schema --- .../dashboards/patchDashboard.int.test.ts | 59 +++++++++++++++++++ .../dashboards/saveDashboard.int.test.ts | 44 ++++++++------ .../api/src/mcp/prompts/dashboards/content.ts | 2 +- .../api/src/mcp/tools/dashboards/schemas.ts | 13 +++- 4 files changed, 97 insertions(+), 21 deletions(-) diff --git a/packages/api/src/mcp/__tests__/dashboards/patchDashboard.int.test.ts b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.int.test.ts index 4b498562ef..ed4f69b7a0 100644 --- a/packages/api/src/mcp/__tests__/dashboards/patchDashboard.int.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/patchDashboard.int.test.ts @@ -655,6 +655,65 @@ describe('MCP Dashboard Tools - clickstack_patch_dashboard', () => { ]); }); + it('should preserve raw SQL number tile colorRules across a get/patch round trip', async () => { + const connectionId = ctx.connection._id.toString(); + const colorRules = [ + { operator: 'lt', value: 0.95, color: 'chart-error', label: 'Down' }, + { operator: 'between', value: [0.95, 0.99], color: 'chart-warning' }, + ]; + const createResult = await callTool( + ctx.client!, + 'clickstack_save_dashboard', + { + name: 'SQL ColorRules Patch Test', + tiles: [ + { + name: 'SLO', + config: { + configType: 'sql', + displayType: 'number', + connectionId, + sqlTemplate: 'SELECT 0.99 AS value LIMIT 1', + color: 'chart-success', + colorRules, + }, + }, + ], + }, + ); + const created = JSON.parse(getFirstText(createResult)); + const tileId = created.tiles[0].id; + + const getDashboardResult = await callTool( + ctx.client!, + 'clickstack_get_dashboard', + { id: created.id }, + ); + const fetched = JSON.parse(getFirstText(getDashboardResult)); + + // Feed the fetched tile straight back in, the way an agent editing an + // existing tile would; the config must survive the round trip intact. + const patchResult = await callTool( + ctx.client!, + 'clickstack_patch_dashboard', + { + dashboardId: created.id, + tileId, + tile: { name: 'SLO (patched)', config: fetched.tiles[0].config }, + }, + ); + expect(patchResult.isError).toBeFalsy(); + + const getResult = await callTool( + ctx.client!, + 'clickstack_get_dashboard_tile', + { dashboardId: created.id, tileId }, + ); + const tile = JSON.parse(getFirstText(getResult)); + expect(tile.config.color).toBe('chart-success'); + expect(tile.config.colorRules).toEqual(colorRules); + }); + it('should round-trip backgroundChart: patch then get_dashboard_tile', async () => { const sourceId = ctx.traceSource._id.toString(); const createResult = await callTool( diff --git a/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts index 760a6b8556..ebc9ceb9fb 100644 --- a/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts @@ -816,34 +816,42 @@ describe('MCP Dashboard Tools - clickstack_save_dashboard', () => { expect(result.isError).toBe(true); }); - it('should keep color but drop colorRules on a raw SQL number tile', async () => { + it('should round-trip color and colorRules on a raw SQL number tile', async () => { const connectionId = ctx.connection._id.toString(); + const sqlConfig = { + configType: 'sql', + displayType: 'number', + connectionId, + sqlTemplate: 'SELECT 0.99 AS value LIMIT 1', + color: 'chart-success', + colorRules: [ + { operator: 'lt', value: 0.95, color: 'chart-error', label: 'Down' }, + { operator: 'between', value: [0.95, 0.99], color: 'chart-warning' }, + ], + }; + const saveResult = await callTool( ctx.client!, 'clickstack_save_dashboard', { name: 'SQL Number Color', - tiles: [ - { - name: 'SLO', - config: { - configType: 'sql', - displayType: 'number', - connectionId, - sqlTemplate: 'SELECT 0.99 AS value LIMIT 1', - color: 'chart-success', - colorRules: [ - { operator: 'gte', value: 1, color: 'chart-error' }, - ], - }, - }, - ], + tiles: [{ name: 'SLO', config: sqlConfig }], }, ); expect(saveResult.isError).toBeFalsy(); const saved = JSON.parse(getFirstText(saveResult)); - expect(saved.tiles[0].config.color).toBe('chart-success'); - expect(saved.tiles[0].config.colorRules).toBeUndefined(); + expect(saved.tiles[0].config).toMatchObject(sqlConfig); + + const getResult = await callTool( + ctx.client!, + 'clickstack_get_dashboard', + { + id: saved.id, + }, + ); + expect(getResult.isError).toBeFalsy(); + const fetched = JSON.parse(getFirstText(getResult)); + expect(fetched.tiles[0].config).toMatchObject(sqlConfig); }); it('should round-trip number-tile backgroundChart through save and get', async () => { diff --git a/packages/api/src/mcp/prompts/dashboards/content.ts b/packages/api/src/mcp/prompts/dashboards/content.ts index 036ad2ee9e..d94c7f94de 100644 --- a/packages/api/src/mcp/prompts/dashboards/content.ts +++ b/packages/api/src/mcp/prompts/dashboards/content.ts @@ -1060,7 +1060,7 @@ Colors are palette tokens, not hex. Order rules from least to most severe so the ] } -colorRules is for number builder tiles only. Raw SQL number tiles (configType: "sql") support color but not colorRules. +Both color and colorRules work on builder number tiles and on raw SQL number tiles (configType: "sql"). == NUMBER TILE BACKGROUND CHART == diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index d1ea8c4a12..1a1740635b 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -127,8 +127,12 @@ const numberTileColorRulesDescription = const rawSqlNumberTileColorDescription = 'Static color for the displayed number, as a palette token such as ' + '"chart-blue" or "chart-success". Valid only when displayType is ' + - '"number", ignored otherwise. Raw SQL number tiles do not support ' + - 'conditional colorRules.'; + '"number", ignored otherwise. Applied unless a colorRules entry matches ' + + 'the value.'; + +const rawSqlNumberTileColorRulesDescription = + `${numberTileColorRulesDescription} Valid only when displayType is ` + + '"number", ignored otherwise.'; const numberTileBackgroundChartDescription = 'Optional background trend sparkline drawn behind the number, derived ' + @@ -1052,6 +1056,11 @@ GROUP BY ServiceName, ts color: ChartPaletteTokenSchema.optional().describe( rawSqlNumberTileColorDescription, ), + colorRules: z + .array(NumberTileColorConditionSchema) + .max(10) + .optional() + .describe(rawSqlNumberTileColorRulesDescription), onClick: mcpOnClickSchema.optional(), }), });