Skip to content

Commit 5c81a86

Browse files
alex-fedotyevclaude
andcommitted
fix(api): expose alternateRowBackground on raw SQL table tiles via MCP
The dashboard authoring MCP schema declared alternateRowBackground on builder table tiles but not on raw SQL table tiles, so save_dashboard and patch_dashboard silently stripped the flag from raw SQL tables even though the UI and REST API both accept it. - Declare alternateRowBackground on the raw SQL MCP tile schema - Add REST raw SQL table omit / explicit-false round-trip coverage - Add MCP raw SQL table save + patch round-trip coverage for the flag - Add MCP-to-REST table schema parity tests (builder + raw SQL) - Use an explicit property for the field in the builder table converter so a rename is a compile error, not a silent runtime drop - List the MCP tile schemas in the shared-schema sync checklist comment Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a49447d commit 5c81a86

7 files changed

Lines changed: 120 additions & 7 deletions

File tree

packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@ describe('MCP Dashboard Tools - clickstack_save_dashboard', () => {
426426
displayType: 'table' as const,
427427
connectionId,
428428
sqlTemplate: 'SELECT 1 AS value LIMIT 1',
429+
alternateRowBackground: true,
429430
},
430431
},
431432
],
@@ -441,6 +442,13 @@ describe('MCP Dashboard Tools - clickstack_save_dashboard', () => {
441442
(t: { name: string }) => t.name === 'SLO',
442443
);
443444
expect(savedSqlNumberTile.config).toMatchObject(sqlNumberConfig);
445+
// Raw SQL table tile: the zebra-stripe flag must survive the MCP save
446+
// path. It was silently stripped before the raw SQL MCP tile schema
447+
// declared alternateRowBackground.
448+
const savedSqlTableTile = saved.tiles.find(
449+
(t: { name: string }) => t.name === 'Other Tile',
450+
);
451+
expect(savedSqlTableTile.config.alternateRowBackground).toBe(true);
444452

445453
const getResult = await callTool(
446454
ctx.client!,
@@ -492,6 +500,12 @@ describe('MCP Dashboard Tools - clickstack_save_dashboard', () => {
492500
(t: { name: string }) => t.name === 'SLO',
493501
);
494502
expect(refetchedSqlNumberTile.config).toMatchObject(sqlNumberConfig);
503+
// "Other Tile" was the one updated (new sqlTemplate); its zebra-stripe
504+
// flag must survive the MCP update (patch) path too.
505+
const refetchedSqlTableTile = refetched.tiles.find(
506+
(t: { name: string }) => t.name === 'Other Tile',
507+
);
508+
expect(refetchedSqlTableTile.config.alternateRowBackground).toBe(true);
495509
});
496510

497511
it('should create a dashboard with a heatmap tile on a Trace source', async () => {
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// The MCP dashboard authoring tile schemas (what save_dashboard / patch_dashboard
2+
// accept) are a hand-maintained mirror of the external REST dashboard schemas,
3+
// with no shared type or derivation. A field added to a REST table config but not
4+
// to the matching MCP tile schema is silently stripped on MCP writes, which is the
5+
// failure mode that let alternateRowBackground drop on raw SQL table tiles. These
6+
// tests assert each MCP table config declares every field its REST counterpart
7+
// accepts, so field-presence drift fails here instead of escaping review.
8+
import {
9+
mcpSqlTileSchema,
10+
mcpTableTileSchema,
11+
} from '@/mcp/tools/dashboards/schemas';
12+
import {
13+
externalDashboardTableChartConfigSchema,
14+
externalDashboardTableRawSqlChartConfigSchema,
15+
} from '@/utils/zod';
16+
17+
describe('MCP dashboard tile schema parity with the external REST schemas', () => {
18+
it('builder table: MCP config declares every field the REST builder table schema accepts', () => {
19+
const restFields = Object.keys(
20+
externalDashboardTableChartConfigSchema.shape,
21+
);
22+
const mcpFields = new Set(
23+
Object.keys(mcpTableTileSchema.shape.config.shape),
24+
);
25+
26+
expect(restFields.filter(field => !mcpFields.has(field))).toEqual([]);
27+
});
28+
29+
it('raw SQL table: MCP config declares every field the REST raw SQL table schema accepts', () => {
30+
const restFields = Object.keys(
31+
externalDashboardTableRawSqlChartConfigSchema.shape,
32+
);
33+
const mcpFields = new Set(Object.keys(mcpSqlTileSchema.shape.config.shape));
34+
35+
expect(restFields.filter(field => !mcpFields.has(field))).toEqual([]);
36+
});
37+
});

packages/api/src/mcp/tools/dashboards/schemas.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,7 @@ const mcpBarTileSchema = mcpTileLayoutSchema.extend({
655655
}),
656656
});
657657

658-
const mcpTableTileSchema = mcpTileLayoutSchema.extend({
658+
export const mcpTableTileSchema = mcpTileLayoutSchema.extend({
659659
config: z.object({
660660
...rejectedTileWhereFields,
661661
displayType: z.literal('table').describe('Tabular aggregated data'),
@@ -947,7 +947,7 @@ const mcpMarkdownTileSchema = mcpTileLayoutSchema.extend({
947947
}),
948948
});
949949

950-
const mcpSqlTileSchema = mcpTileLayoutSchema.extend({
950+
export const mcpSqlTileSchema = mcpTileLayoutSchema.extend({
951951
config: z.object({
952952
configType: z
953953
.literal('sql')
@@ -1023,6 +1023,13 @@ GROUP BY ServiceName, ts
10231023
color: ChartPaletteTokenSchema.optional().describe(
10241024
rawSqlNumberTileColorDescription,
10251025
),
1026+
alternateRowBackground: z
1027+
.boolean()
1028+
.optional()
1029+
.describe(
1030+
'Zebra-stripe the table by tinting alternating rows, which aids scanning on wide tables. ' +
1031+
'Valid only when displayType is "table", ignored otherwise. Default false.',
1032+
),
10261033
onClick: mcpOnClickSchema.optional(),
10271034
}),
10281035
});

packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,6 +2789,57 @@ describe('External API v2 Dashboards - new format', () => {
27892789
);
27902790
});
27912791

2792+
it('omits alternateRowBackground on a raw SQL table tile when not provided, and persists explicit false', async () => {
2793+
const connectionId = connection._id.toString();
2794+
const sourceId = traceSource._id.toString();
2795+
const sqlTemplate = 'SELECT count() FROM otel_logs WHERE {timeFilter}';
2796+
2797+
const tableNoStripe: ExternalDashboardTile = {
2798+
name: 'Raw SQL table without stripe setting',
2799+
x: 0,
2800+
y: 0,
2801+
w: 6,
2802+
h: 3,
2803+
config: {
2804+
configType: 'sql',
2805+
displayType: 'table',
2806+
connectionId,
2807+
sqlTemplate,
2808+
sourceId,
2809+
},
2810+
};
2811+
2812+
const tableStripeOff: ExternalDashboardTile = {
2813+
name: 'Raw SQL table with stripe explicitly off',
2814+
x: 6,
2815+
y: 0,
2816+
w: 6,
2817+
h: 3,
2818+
config: {
2819+
configType: 'sql',
2820+
displayType: 'table',
2821+
connectionId,
2822+
sqlTemplate,
2823+
sourceId,
2824+
alternateRowBackground: false,
2825+
},
2826+
};
2827+
2828+
const response = await authRequest('post', BASE_URL)
2829+
.send({
2830+
name: 'Dashboard raw SQL table stripe defaults',
2831+
tiles: [tableNoStripe, tableStripeOff],
2832+
})
2833+
.expect(200);
2834+
2835+
expect(response.body.data.tiles[0].config).not.toHaveProperty(
2836+
'alternateRowBackground',
2837+
);
2838+
expect(response.body.data.tiles[1].config.alternateRowBackground).toBe(
2839+
false,
2840+
);
2841+
});
2842+
27922843
// Schema-level rejections that exercise pure Zod constraints
27932844
// (discriminated-union absence, `min(1)` on valueExpression, and
27942845
// `length(1)` on the select array). The non-Trace-source case

packages/api/src/routers/external-api/v2/utils/dashboards.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,11 +423,13 @@ const convertToExternalTileChartConfig = (
423423
'having',
424424
'numberFormat',
425425
'groupByColumnsOnLeft',
426-
'alternateRowBackground',
427426
'onClick',
428427
]),
429428
displayType: config.displayType,
430429
sourceId,
430+
// Explicit property (not via `pick`) so a future rename is a compile
431+
// error rather than a silent runtime drop; matches the raw SQL arm.
432+
alternateRowBackground: config.alternateRowBackground,
431433
asRatio:
432434
config.seriesReturnType === 'ratio' &&
433435
Array.isArray(config.select) &&

packages/api/src/utils/zod.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ const externalDashboardBarRawSqlChartConfigSchema =
332332
seriesLimit: z.number().int().nonnegative().optional(),
333333
});
334334

335-
const externalDashboardTableChartConfigSchema = z.object({
335+
export const externalDashboardTableChartConfigSchema = z.object({
336336
displayType: z.literal('table'),
337337
sourceId: objectIdSchema,
338338
select: z.array(externalDashboardSelectItemSchema).min(1).max(20),
@@ -348,7 +348,7 @@ const externalDashboardTableChartConfigSchema = z.object({
348348
showOperandSeries: externalShowOperandSeriesSchema,
349349
});
350350

351-
const externalDashboardTableRawSqlChartConfigSchema =
351+
export const externalDashboardTableRawSqlChartConfigSchema =
352352
externalDashboardRawSqlChartConfigBaseSchema.extend({
353353
displayType: z.literal('table'),
354354
alternateRowBackground: z.boolean().optional(),

packages/common-utils/src/types.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,9 +1416,11 @@ export const BackgroundChartSchema = z.object({
14161416
export type BackgroundChart = z.infer<typeof BackgroundChartSchema>;
14171417

14181418
// When making changes here, consider if they need to be made to the external API
1419-
// as well: the Zod schema (packages/api/src/utils/zod.ts) and the hand-written
1419+
// as well: the Zod schema (packages/api/src/utils/zod.ts), the hand-written
14201420
// OpenAPI JSDoc (packages/api/src/routers/external-api/v2/dashboards.ts), which
1421-
// duplicates this shape for the generated spec.
1421+
// duplicates this shape for the generated spec, and the MCP dashboard authoring
1422+
// tile schemas (packages/api/src/mcp/tools/dashboards/schemas.ts), a separate
1423+
// hand-maintained mirror that gates what save_dashboard / patch_dashboard accept.
14221424
/**
14231425
* Schema describing settings which are shared between Raw SQL
14241426
* chart configs and Structured ChartBuilder chart configs

0 commit comments

Comments
 (0)