[HDX-5081] Formula support across API surfaces - #2952
Conversation
🦋 Changeset detectedLatest commit: ddfd42d The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThe PR extends formula support through external dashboard APIs, MCP dashboard tools, alert evaluation, and CLI chart rendering.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the MCP patch path now rejects invalid formula configurations before persistence, and unchanged formula tiles no longer block unrelated dashboard updates after a source-kind change.
|
| Filename | Overview |
|---|---|
| packages/api/src/mcp/tools/dashboards/patchDashboard.ts | Revalidates merged tile patches with the external schema before shared validation and persistence, resolving the prior formula-validation bypass. |
| packages/api/src/routers/external-api/v2/utils/dashboards.ts | Round-trips formula fields and limits update-time source-kind validation to new or changed formula tiles. |
| packages/api/src/utils/zod.ts | Adds formula fields and shared cross-field validation to supported external builder tile schemas. |
| packages/common-utils/src/dashboardValidation.ts | Centralizes formula parsing, operand-reference, asRatio, and number-tile validation. |
| packages/api/src/mcp/tools/dashboards/schemas.ts | Exposes formula configuration in MCP schemas while leaving cross-field enforcement to the external tile schema. |
| packages/cli/src/shared/tileConfig.ts | Delegates number and table transformations to shared formula-aware chart utilities. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Input[External API or MCP tile input] --> Schema[External tile schema]
Schema --> FormulaValidation[Shared formula validation]
FormulaValidation --> SourceGate[Formula source-kind gate]
SourceGate --> Persist[(Dashboard configuration)]
Persist --> Query[Metric or event query path]
Query --> FormulaResult[Computed formula series]
Persist --> Alerts[Alert evaluation]
Persist --> CLI[CLI chart rendering]
Reviews (9): Last reviewed commit: "chore: simplify comments and drop ticket..." | Re-trigger Greptile
E2E Test Results✅ All tests passed • 308 passed • 1 skipped • 1058s
Tests ran across 4 shards in parallel. |
🔴 Tier 4 — CriticalTouches 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:
Review process: Deep review from a domain expert. Synchronous walkthrough may be required. Stats
|
Deep Review✅ No critical issues found. This is a well-constructed, contract-disciplined change. Formula validation is single-sourced through 🟡 P2 — recommended
🔵 P3 nitpicks (12)
Reviewers (9): correctness, security, api-contract, testing, maintainability, project-standards, agent-native, kieran-typescript, learnings-researcher. The adversarial reviewer was dispatched but did not return (output stalled shortly after launch); its scope — round-trip fidelity, source-kind-gate bypass, and number-tile edge cases — was independently spot-checked during synthesis and corroborated the security/correctness conclusions (no injection path; gate is a feature gate, not a tenant boundary; number-tile select branching has no data-loss path). Testing gaps:
Note: OpenAPI regeneration byte-sync could not be verified in-environment ( |
37994e2 to
a8973eb
Compare
a8973eb to
7a82f60
Compare
jordan-simonovski
left a comment
There was a problem hiding this comment.
LGTM! Nice work on this one.
7a82f60 to
ec02019
Compare
da931d9 to
223d1db
Compare
223d1db to
aec00a7
Compare
Plumb metric formula configs through every surface that persists or accepts chart configs: - External dashboards API v2: formulas/showOperandSeries on line, stacked_bar, table and number builder tile schemas, round-tripped through both converters, with write-time expression validation and a metric-source-only gate. OpenAPI spec regenerated with the new Formula component. - Shared validation: validateChartConfigFormulas in common-utils dashboardValidation mirrors the chart editor's save-time rules; isFormulaDisplayType moves to common-utils core/utils so the editor and server gates cannot drift. - MCP: save_dashboard / patch_dashboard tile schemas accept formulas, query_tile computes formula columns, and the dashboards query-guide prompt documents the feature. - CLI: number/table tile config transforms delegate to the shared formula-aware common-utils implementations; AGENTS.md pipeline notes updated. Alert-on-formula-tile support and its checkAlerts integration tests landed with HDX-5080 (#2909); verified green on this branch.
…e gate Address two review findings on HDX-5081: - clickstack_patch_dashboard now re-validates the merged tile through externalDashboardTileSchemaWithId, the same schema the save_dashboard body schemas route every tile through. Previously a patch could persist configs an equivalent create/full-update rejects: malformed or unknown-ref formula expressions, asRatio combined with formulas (also a pre-existing gap for the asRatio/two-select rule), multiple formulas on a number tile, and multi-select number tiles without formulas. - The metric-source formula gate in validateDashboardTiles is now scoped on updates via filterChangedFormulaTiles, mirroring the heatmap gate: a tile that already carried the same formulas on the same source no longer blocks unrelated dashboard edits when the source's kind was changed after acceptance. New formula tiles, edited formulas, and sourceId changes still flow through the check.
Rebased onto #2953 (event-source formulas) and widened the API gate to match its editor gating: - isFormulaSourceKind (Metric | Log | Trace) moves from the app chart editor into common-utils core/utils (the app re-exports it), so the editor's Add Formula gating and the server-side surfaces share one predicate. - The external/MCP formula source gate now rejects only formula-incapable kinds (e.g. session) instead of everything non-metric, with an updated error message; MCP tool descriptions, the dashboards query-guide prompt, and the OpenAPI spec document the widened support (including a log error-rate example). - Tests: external round-trip of a trace-source formula tile plus a session-source rejection (kind-flip test now flips to session); MCP save + query_tile of a log-source formula tile asserting the inline-computed value with hidden operands; checkAlerts event-formula tile alert proving the threshold evaluates the formula value.
aec00a7 to
ddfd42d
Compare
Summary
Plumbs chart formula configs (HDX-5078/5079/5080/5132) through every remaining surface that persists or accepts chart configs, so formulas created anywhere round-trip and query correctly everywhere — on metric and log/trace event sources.
External dashboards API v2
formulas(letter-ref arithmetic over the tile's select items, e.g.A / (A + B) * 100) andshowOperandSeriesare accepted on line / stacked_bar / table / number builder tiles and round-trip through POST → GET → PUT.showOperandSeries: falseinternally (never exposed on the external surface).validateChartConfigFormulashelper in common-utilsdashboardValidation(same rules as the chart editor'svalidateChartForm): expression parse + series-ref range checks,asRatiomutual exclusion, number-tile single-formula cap.isFormulaSourceKindpredicate (moved from the chart editor into common-utilscore/utils, app re-exports — the editor gating and the server gates cannot drift). Other kinds (e.g. session) are rejected. On updates the gate is scoped to changed formula tiles (mirroring the heatmap gate), so a source whosekindchanged after acceptance doesn't block unrelated dashboard edits.Formulacomponent schema.MCP
clickstack_save_dashboard/clickstack_patch_dashboardtile schemas declareformulas/showOperandSerieswith LLM-facing descriptions (previously the strict Zod schemas silently stripped them).clickstack_patch_dashboardre-validates the merged tile through the external tile schema, so patches can no longer persist configs an equivalent create/full-update rejects (also closes a pre-existing bypass of the asRatio/two-select rule).clickstack_query_tilecomputes formula columns for both the composed metric path and the inline event path.== FORMULAS ==section with metric and log/trace examples.Alerts
checkAlertsint test for an event (log) formula tile proving the threshold evaluates the formula value (25 = 1 error / 4 total × 100), not an operand.CLI
shared/tileConfig.tsdelegated its stale localconvertToNumberChartConfig/convertToTableChartConfigports to the shared formula-aware common-utils implementations (the web already imports these), so formula tiles render identically to the web — including operand hiding on number tiles.packages/cli/AGENTS.mdpipeline notes updated.How to test on Vercel preview
N/A — non-UI change (API/MCP/CLI surfaces only; the
packages/appedits are behavior-identical re-exports).Testing
make ci-lint,make ci-unit— green across all packages (incl. [HDX-5132] Formula support for log/trace event sources #2953's editor tests, which now consume the re-exported predicates).make dev-int):external-api/__tests__/dashboards.int.test.ts— 187 passed, incl. aMetric formulas (HDX-5081)suite: per-display-type round-trips (metric + trace event source), PUT preservation, source-kind-flip scoping, and rejections (unknown series ref, malformed expression,asRatio+ formulas, multiple formulas on number, multi-select number without formulas, formulas on a session source).mcp/__tests__/dashboards— 131 passed, incl. saving formula tiles through MCP,query_tilecomputing the metric formula (10 / 200 × 100 = 5) and the log event formula (3 / 12 × 100 = 25) with operands hidden, patch-path validation parity, and session-source rejection.tasks/checkAlerts/__tests__/checkAlerts.int.test.ts— 173 passed, incl. the new event-formula tile alert.validateChartConfigFormulas.References