Skip to content

feat(api): expose table tile alternateRowBackground in the external Dashboards API - #2698

Open
alex-fedotyev wants to merge 4 commits into
mainfrom
alex/HDX-4602-table-tile-alternate-row-external-api
Open

feat(api): expose table tile alternateRowBackground in the external Dashboards API#2698
alex-fedotyev wants to merge 4 commits into
mainfrom
alex/HDX-4602-table-tile-alternate-row-external-api

Conversation

@alex-fedotyev

@alex-fedotyev alex-fedotyev commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #2519, which added the Alternate Row Background display setting (zebra striping) to table tiles in the app and the internal chart schema. That PR shipped the app and common-utils slice only; this brings the setting to the external REST Dashboards API and the MCP dashboard authoring tool.

The field lives on the shared chart-settings schema rather than the builder-only config, so it applies to both builder and raw SQL table tiles (we moved it up to the shared schema during the #2519 review so SQL tiles get striping too). The external API now matches:

  • Adds alternateRowBackground (optional boolean) to both the builder table config and the raw SQL table config in the request/response schemas (zod.ts).
  • Threads it through the internal/external converters in both directions, guarding the raw SQL side by display type exactly like the existing onClick field.
  • Documents it in the OpenAPI spec for both TableBuilderChartConfig and TableRawSqlChartConfig, and regenerates openapi.json.

It also exposes the field to the clickstack_save_dashboard MCP tool so AI-authored dashboards can turn on striping:

  • Adds the optional boolean to mcpTableTileSchema.config, next to groupByColumnsOnLeft (schemas.ts).
  • Appends one guidance sentence to the existing inventory-style-tables prompt rule (content.ts). The MCP save path validates and converts through the external dashboard schema, so it rides on the same converter change above.

No new helpers; every edit sits beside the existing groupByColumnsOnLeft / onClick fields on the same schema, converter, and JSDoc block.

Open question: expose it on the raw SQL MCP tile schema too?

I kept the MCP side to the builder table tile (mcpTableTileSchema) for now. alternateRowBackground lives on the shared chart-settings schema, so the external API above exposes it on both the builder and raw SQL table configs, and the app (from #2519) applies striping to raw SQL table tiles as well. mcpSqlTileSchema already carries numberFormat, color, and onClick, so adding it there would mirror onClick exactly (a table-only field carried table-guarded through the external round-trip). I left it out to keep the MCP SQL tile schema lean, but I'm happy to add it for full parity. Your call.

Test plan

  • Extended the POST and PUT round-trip integration tests to send and assert alternateRowBackground: true on both a builder table tile and a raw SQL table tile. The raw SQL assertions are new coverage that guards the SQL path.
  • dashboards.int.test.ts: 162/162 passing locally against a ClickHouse + MongoDB stack. This is the end-to-end round-trip proof (POST a dashboard with the field, read it back, field preserved on both config kinds).
  • mcp/__tests__/dashboards/saveDashboard.int.test.ts: 59/59 passing; the multi-tile create round-trip asserts alternateRowBackground: true survives on the returned builder table tile.
  • tsc --noEmit, eslint, and spectral (lint:openapi) all clean.

Tier

This lands review/tier-4 because it necessarily touches packages/api/src/routers/external-api/, which the triage classifier always treats as critical-path. The change itself is small (47 production lines) and mirrors the existing groupByColumnsOnLeft and onClick fields.

Not in this PR

  • ClickStack customer docs (DOC-855).

@vercel

vercel Bot commented Jul 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview Aug 27, 2026 11:02pm
hyperdx-storybook Ready Ready Preview Aug 27, 2026 11:02pm

Request Review

@changeset-bot

changeset-bot Bot commented Jul 21, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5c81a86

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/api Patch
@hyperdx/app Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

🔴 Tier 4 — Critical

Touches authentication, tenancy data models, the public API or shipped database config — or substantially changes the query rendering engine, background tasks, the OTel pipeline, image build, or release CI.

Why this tier:

  • Critical-path files (2) — tenancy, public API, or shipped database config:
    • packages/api/src/routers/external-api/v2/dashboards.ts
    • packages/api/src/routers/external-api/v2/utils/dashboards.ts

Additional context: spans packages but only 70 prod lines — under the 100-line cross-layer bar

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats
  • Production files changed: 7
  • Production lines changed: 70 (+ 167 in test files, excluded from tier calculation)
  • Critical-path lines changed: 26
  • Branch: alex/HDX-4602-table-tile-alternate-row-external-api
  • Author: alex-fedotyev

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions github-actions Bot added the review/tier-4 Critical — deep review + domain expert sign-off label Jul 21, 2026
@greptile-apps

greptile-apps Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR exposes table zebra striping through the external dashboard API and MCP dashboard authoring path. The main changes are:

  • Adds alternateRowBackground to builder and raw SQL table schemas.
  • Threads the field through dashboard config conversion in both directions.
  • Updates OpenAPI output and JSDoc for the public API.
  • Adds MCP schema support and parity coverage.
  • Extends dashboard round-trip tests for omitted, false, and true values.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
packages/api/src/routers/external-api/v2/utils/dashboards.ts Adds conversion support for alternateRowBackground on builder and raw SQL table configs.
packages/api/src/utils/zod.ts Adds the optional field to the external builder and raw SQL table config schemas.
packages/api/src/mcp/tools/dashboards/schemas.ts Adds MCP schema support for table zebra striping on builder and raw SQL tiles.
packages/api/src/mcp/tests/dashboards/schema-parity.test.ts Adds coverage to keep MCP table schemas aligned with the REST table schemas.
packages/api/src/routers/external-api/tests/dashboards.int.test.ts Extends dashboard round-trip coverage for the new table striping field.

Reviews (5): Last reviewed commit: "fix(api): expose alternateRowBackground ..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 323 passed • 1 skipped • 1301s

Status Count
✅ Passed 323
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

This is a small, purely additive change: an optional alternateRowBackground boolean threaded onto the external REST Dashboards table configs (builder + raw SQL) and the MCP dashboard authoring tile schemas, mirroring the existing groupByColumnsOnLeft and onClick fields. The converters, schema mirrors, and OpenAPI docs were verified consistent, and the round-trip is covered by integration tests on both config kinds.

Verified during review:

  • packages/api/src/routers/external-api/v2/utils/dashboards.ts — both conversion directions plumb the field correctly. Internal→external emits it only within the Table arm (builder line 432, raw SQL line 241); external→internal guards the raw SQL arm on displayType === 'table' (lines 730–733) exactly like onClick/color, and the trailing _.omitBy(_.isNil) drops the undefined for non-table types. Explicit false survives (_.isNil matches null/undefined only); an absent field serializes away over HTTP JSON. Builder table adds the key to the pick list plus an explicit property so a rename fails at compile time.
  • packages/api/src/utils/zod.tsz.boolean().optional() added to both externalDashboardTableChartConfigSchema and externalDashboardTableRawSqlChartConfigSchema, matching the groupByColumnsOnLeft pattern (optional, no .default()).
  • packages/api/src/routers/external-api/v2/dashboards.ts + packages/api/openapi.json — hand-written JSDoc for TableBuilderChartConfig and TableRawSqlChartConfig matches the Zod fields, and the regenerated spec is consistent with the JSDoc for both configs. Additive optional field ⇒ backward compatible, no version bump needed.
  • packages/api/src/mcp/tools/dashboards/schemas.ts — field declared on both mcpTableTileSchema and mcpSqlTileSchema; the raw SQL description correctly scopes it to displayType: table. The new schema-parity.test.ts asserts each MCP table config declares every field its REST counterpart accepts, which fails CI on future field-presence drift.
  • Testsdashboards.int.test.ts and saveDashboard.int.test.ts cover create and update round-trips on both builder and raw SQL table tiles, including the omit-vs-explicit-false distinction. The raw SQL MCP assertions guard the exact path a prior review flagged as silently stripping the field.

Prior-review status: every finding from the earlier deep review — the P0/P1 raw SQL MCP schema strip, the omit/explicit-false REST coverage, the MCP↔REST parity test, the changeset, the sync-checklist comment, and the explicit-property converter change — is genuinely resolved in this diff, not merely claimed.

One benign behavior worth noting (not a defect): the MCP raw SQL tile schema accepts alternateRowBackground for any displayType, and for non-table raw SQL tiles the field is silently stripped by the non-strict external raw SQL schema on save rather than rejected. This matches the field's documented "ignored otherwise" semantics and mirrors how onClick already behaves.


Reviewers (8): correctness, adversarial, api-contract, testing, maintainability, kieran-typescript, project-standards, previous-comments. Confirmed-clean returns from correctness and api-contract; findings synthesized with an orchestrator-level trace of both converter directions, the four schema mirrors, and the integration coverage.

Testing gaps: none substantive — create/update round-trips and omit/explicit-false are covered on both builder and raw SQL table configs, and drift is guarded by schema-parity.test.ts.

@alex-fedotyev

Copy link
Copy Markdown
Contributor Author

Addressed the deep-review findings in cec1359.

P0/P1 (raw SQL MCP tile schema strips alternateRowBackground): fixed. mcpSqlTileSchema.config now declares the field, so save_dashboard / patch_dashboard preserve it on raw SQL table tiles. The earlier commit had only added it to the builder table schema, which is why the raw SQL path still dropped it.

P2 (raw SQL omit vs explicit-false REST coverage): added a POST test in dashboards.int.test.ts that sends one raw SQL table tile omitting the field (asserts it is absent on read) and one with alternateRowBackground: false (asserts false), mirroring the builder assertions.

P2 (MCP to REST schema parity): added schema-parity.test.ts, which asserts each MCP table config (builder and raw SQL) declares every field its REST counterpart accepts. This fails CI on field-presence drift instead of leaving it to review. It would have failed on this exact gap before the P0/P1 fix.

P2 (changeset): the branch already carries table-tile-alternate-row-external-api.md and table-tile-alternate-row-mcp.md. Kept at patch to match the fixed-group convention used by the other external dashboards API additions (heatmap, containers/tabs).

P3 (sync-checklist comment): added the MCP tile schemas to the mirror list above SharedChartSettingsSchema in common-utils/types.ts, since that was the mirror that drifted.

P3 (unguarded read passthrough): left as is. alternateRowBackground reads the same way as groupByColumnsOnLeft and having, which are also unvalidated on read; the loud 400 on a malformed out-of-band value is the established behaviour, so guarding only this field would be inconsistent.

P3 (explicit property vs lodash pick): the builder table converter now sets alternateRowBackground as an explicit property, so a rename is a compile error rather than a silent runtime drop, matching the raw SQL arm.

Also seeded a raw SQL table tile with alternateRowBackground: true in the existing MCP save/get/update/re-get round-trip in saveDashboard.int.test.ts and assert it survives both the save and the update, which is the MCP-side regression test flagged as a testing gap.

Local: lint and typecheck clean, 580 api unit tests pass including the new parity tests. The REST and MCP integration coverage runs in the integration CI job.

alex-fedotyev and others added 4 commits August 27, 2026 22:48
…ashboards API

Follow-up to #2519, which added the Alternate Row Background display setting to table tiles in the app and internal schema. That field lives on the shared chart-settings schema, so it applies to both builder and raw SQL table tiles.

This exposes it in the external REST Dashboards API on both the builder and raw SQL table chart configs, alongside the OpenAPI docs, so API consumers can set striping wherever the UI allows it. Round-trip integration coverage is extended for POST and PUT on both config kinds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a table-tile case asserting that omitting alternateRowBackground leaves it absent on read-back, and that an explicit false persists, mirroring the existing omits-orderBy coverage. Addresses a review nit on the field's test coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oring schema

Adds alternateRowBackground to the clickstack_save_dashboard builder table tile schema and a short prompt guidance line, with a handler round-trip test asserting the field persists. Depends on the external API PR it is stacked on, since the MCP save path validates and converts through the external dashboard schema.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-4 Critical — deep review + domain expert sign-off

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant