feat(mcp): gauge chart type plugin - #43568
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43568 +/- ##
=======================================
Coverage 79.24% 79.25%
=======================================
Files 2888 2889 +1
Lines 166446 166543 +97
Branches 38529 38548 +19
=======================================
+ Hits 131908 131996 +88
- Misses 32045 32059 +14
+ Partials 2493 2488 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8811240 to
8fe90c2
Compare
dosubot
Bot
added
change:backend
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| None, description="Minimum value of the dial scale (default: auto)" | ||
| ) | ||
| max_val: float | None = Field( | ||
| None, description="Maximum value of the dial scale (default: auto)" |
There was a problem hiding this comment.
Suggestion: Validate that the configured scale bounds are ordered before mapping them to form_data. As written, a request with min_val greater than max_val is accepted and reaches the frontend, where the negative scale range produces invalid or misleading gauge axis calculations. Reject reversed bounds with a model validator. [incorrect condition logic]
Severity Level: Major ⚠️
- ⚠️ Gauge charts can display inverted axis ranges.
- ⚠️ Configured minimum and maximum semantics are not enforced.
- ⚠️ Gauge interval colors may use incorrect bounds.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/mcp_service/chart/schemas.py
**Line:** 1073:1076
**Comment:**
*Incorrect Condition Logic: Validate that the configured scale bounds are ordered before mapping them to `form_data`. As written, a request with `min_val` greater than `max_val` is accepted and reaches the frontend, where the negative scale range produces invalid or misleading gauge axis calculations. Reject reversed bounds with a model validator.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
The suggestion to validate that superset/mcp_service/chart/schemas.py |
Code Review Agent Run #262b50Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
There was a problem hiding this comment.
Thanks for this — it's a clean, well-structured plugin and one of seven in the family, so I've split findings into gauge-specific and template-level (all 7). Everything below was verified against head 8fe90c2a; I ran the unit tests and executed the repros noted as RAN.
What's genuinely good here
- The plugin contract is faithful to the frontend.
map_gauge_configmirrors theGauge/buildQuery+controlPanelcontract precisely:groupby → one dial per row,metricviacreate_metric_object, andcolor_schemedefaulting to'supersetColors'.row_limit: Field(10, ge=1, le=10)is correct, not off-by-one — the frontendcontrolPanelcapschoicesto[...Array(10).keys()].map(n => n + 1)(i.e. 1–10), sole=10matches the canonical cap. Nice attention to detail. (RAN:test_gauge_row_limit_capped_at_tenpasses; INSPECTEDGauge/controlPanel.tsx.) - Bad column names are validated, not silently dropped.
normalize_column_refsroutesmetric/groupby/filters throughDatasetValidator.get_canonical_*, so this plugin does not repeat the SC-117852 silent-discard shape for column resolution. Good. - The registry is defensive (
registry.py:154): duplicatechart_typeandnative_viz_typescollisions both log warnings.gauge_chartis a unique literal, so no collision with siblings. - The 2 deleted lines in
schemas.pyare benign — they're the two lines of theChartConfigdiscriminator description string, rewritten to insert'gauge_chart'. No behavioral regression to the shared file.
Template-level (applies to all 7 siblings)
1. The reject_metric_style_groupby guard has a hole: it misses aggregate ⚠️ (highest-value item)
reject_metric_style_groupby rejects sql_expression and saved_metric on a groupby entry, but not aggregate — even though a ColumnRef with aggregate set is metric-like. The result is a silent drop:
# RAN at 8fe90c2a:
GaugeChartConfig(chart_type="gauge_chart",
metric={"name":"x","aggregate":"AVG"},
groupby=[{"name":"team","aggregate":"SUM"}])
# → accepted; map_gauge_config(...)["groupby"] == ["team"] (the SUM is silently discarded)This is the same class codeant flagged on the treemap sibling (#43569) — and I verified the reject_metric_style_groupby validator is byte-identical across gauge, treemap (#43569), and radar (#43571), so the hole is cloned. The repo already has the right predicate: ColumnRef.is_metric (schemas.py:752) returns aggregate or saved_metric or sql_expression, and _metric_display_label treats all three as metric markers. Suggested shared fix: have the guard reject col.is_metric (covering all three markers) rather than checking saved_metric alone. One convention fixes it in all the siblings that have a groupby.
2. Merge-conflict surface: all 7 rewrite the same lines
Every sibling edits the identical ChartConfig discriminator description line ("'pie', 'pivot_table', ..." → inserting its own literal) and inserts into the same import blocks in schemas.py, chart_utils.py, plugins/__init__.py, and get_chart_type_schema.py, plus the same union-member region. (INSPECTED all 6 sibling patches.) Whichever merges first, the other six will hit conflicts on that hunk. Not a defect in this PR — just worth coordinating merge order (or expecting six rebases) now rather than after the fact.
3. On the "schemas encode types but not semantic constraints" synthesis — partially refuted
Each of the 7 configs does ship a @model_validator(mode="after") encoding a semantic constraint, so it's not "types only." The sharper, accurate statement is: the semantic validators are incomplete and copy-pasted — see finding #1. That's the pattern worth fixing convention-wide.
Gauge-specific
4. codeant's inverted-bounds claim — CONFIRMED, but Minor (not Major)
min_val/max_val are bare float | None with no ordering check, and there's no @model_validator covering them (the one that exists only guards groupby).
# RAN at 8fe90c2a:
GaugeChartConfig(chart_type="gauge_chart", metric={"name":"progress","aggregate":"AVG"},
min_val=100, max_val=0)
# → accepted; form_data == {min_val: 100.0, max_val: 0.0}Downstream (traced, not inferred): Gauge/transformProps.ts:245 computes range(min, max, (max - min) / splitNumber). With min>max the step is negative, so lodash range yields a descending axis ([100, 90, …]) and min/max pass straight into the echarts gauge series (:342-343). It renders an inverted/misleading dial — it does not crash or go blank. So "Major" overstates it; I'd call it Minor.
Two caveats that lower the priority further, worth stating so it's judged fairly:
- This is not a fail-open divergence from the canonical path. The frontend
min_val/max_valare plainTextControls with no ordering validation either — the Explore UI acceptsmin>maxand renders the same inverted dial. The MCP layer is mirroring existing behavior, not weakening a check the canonical path enforces. - It's gauge-specific: no sibling has an ordered scalar pair (I checked — the others' fields are all
ColumnRefs or singlerow_limits), so this particular bug does not replicate six more times.
Still a reasonable hardening: a @model_validator(mode="after") rejecting min_val is not None and max_val is not None and min_val >= max_val would give the agent-facing path a guardrail the UI lacks. Low cost, clear message.
5. Test adequacy (Rule 26)
test_gauge_chart.py (+169) is well-targeted — reverting only the production files fails ~all 15 tests (they import GaugeChartConfig/map_gauge_config/the registry). (RAN: 15 passed.) It includes real negative tests for row_limit>10 and groupby saved_metric. But as expected there is no inverted-bounds test (the happy-path test_gauge_form_data_with_dials_and_range uses ordered 0/100) and no aggregate-on-groupby test. If you act on #1 and #4, please add those two negatives.
Minor / nits
get_chart_type_schema.pyexample is metric-only — fine, since theFielddescriptions are strong, but agroupby+min_val/max_valexample would show the multi-dial + scale case to an agent.
Bottom line: the template is sound and worth propagating to the other six — the plugin/registry/mapping structure is solid and the field-level validation is thoughtful. The one thing I'd fix as a shared convention before the family lands is finding #1 (use is_metric in the groupby guard); #4 is a nice-to-have hardening specific to gauge. Not resolving codeant's thread; leaving that to you.
CI at 8fe90c2a, re-derived deduped (latest run per check name): 46 success / 3 neutral / 12 skipped / 0 failures across 61 distinct checks (125 raw runs) + netlify StatusContext success.
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
aminghadersohi
left a comment
aminghadersohi
left a comment
There was a problem hiding this comment.
Round 2 follow-up — reviewed at head e264888d. Both items from my earlier review are addressed, and I verified them by running the suite (RAN: 17 passed).
Both fixes land correctly ✅
groupbymetric-marker gap —reject_metric_style_groupbynow gates oncol.is_metric(schemas.py:751), which unifiesaggregate/saved_metric/sql_expression. This closes theaggregate-on-groupby hole exactly as hoped, using the in-repo predicate rather than an ad-hoc check. RAN:groupby=[{name:'team', aggregate:'SUM'}]now raisesValidationError(previously accepted, SUM silently dropped). The newtest_gauge_groupby_rejects_aggregatecovers it.- Inverted
min_val/max_val(codeant's open thread) — the newreject_inverted_boundsvalidator rejectsmin_val >= max_val. Using>=(not>) is the right call: equal bounds are also degenerate (range(min, max, 0)→ empty axis). RAN:min_val=100, max_val=0now raises;test_gauge_rejects_inverted_boundscovers it. This resolves the substance of codeant's inline suggestion onschemas.py:1076— I'm leaving the thread itself for you to close.
Nice, minimal fixes — both reuse existing conventions and each ships a matching negative test.
One extra check I ran on this family — gauge is clean here
The sibling PRs in this family have a recurring shape where a query transform that lives only in the frontend buildQuery.ts is lost on the MCP path (which builds the query server-side and never runs buildQuery.ts). I checked gauge specifically for it and it does not apply:
min_val/max_valare render-only — they feed the dial axis scale inGauge/transformProps.ts, not the query. Nothing to lose.- Gauge's only
buildQuery.tsquery transform issort_by_metric && { orderby: [[metric, false]] }. Butsort_by_metricis opt-in and off by default in the frontend (no default in the shared control, andGauge/DEFAULT_FORM_DATAdoesn't set it), so the frontend default emits noorderbyeither.map_gauge_configemits noorderby/sort_by_metric— which matches the frontend default. So there's no silent divergence for gauge.
If you ever want to expose the "sort dials by metric" toggle for the high-cardinality-groupby case, that'd be a sort_by_metric field mapped through to the server's sort_by_metric → orderby conversion — but it's a feature, not a gap, and out of scope here.
Code-wise this looks good to me. As an external-contributor PR I'm keeping this a comment rather than a formal approval, but I have no remaining blockers on the gauge plugin itself.
Code Review Agent Run #9e950dActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Adds a generate_chart plugin for the gauge viz type (viz_type 'gauge_chart'). Mirrors the frontend Gauge buildQuery contract: a single metric the dial displays, plus an optional multi groupby that renders one dial per row (capped at the frontend's 10). min_val/max_val fix the dial scale. - GaugeChartConfig schema (metric required; optional groupby, row_limit <=10, min_val/max_val, filters, color_scheme) added to the ChartConfig discriminated union and the get_chart_type_schema adapters. - map_gauge_config maps the config to form_data; saved metrics pass through as a bare name string, ad-hoc metrics as SIMPLE/SQL adhoc objects. - GaugeChartPlugin registered in the plugin registry. - 17 unit tests covering schema validation (optional groupby, row-limit cap, dimension-not-a-metric), union dispatch, form_data mapping, and registry integration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…unds (review) Addresses aminghadersohi's review: - reject_metric_style_groupby gates on ColumnRef.is_metric, so an aggregate on a dial dimension is rejected (previously only saved_metric/sql_expression). - New reject_inverted_bounds validator rejects min_val >= max_val, which would render an inverted/degenerate dial (the Explore UI lacks this guard; this gives the agent-facing path one). - Negative tests for both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds gauge_chart to the tool docstring's core chart_type list so the LLM-facing discovery string matches the registered adapters (aminghadersohi round-3 note). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
e264888 to
6e8ab39
Compare
rebenitez1802
left a comment
rebenitez1802
left a comment
There was a problem hiding this comment.
Faithful, well-tested gauge plugin — the form_data mapping matches the frontend contract exactly — but the two docstrings LLMs actually read to discover chart types never mention gauge, which undercuts the PR's own goal of making gauge reachable via MCP.
🟡 Medium — generate_chart docstring never lists gauge_chart
The generate_chart tool docstring (superset/mcp_service/chart/tool/generate_chart.py:88-90) enumerates the valid chart_type values ('xy', 'table', 'pie', … 'box_plot', 'waterfall', "plus host-gated types") and gauge is absent — and gauge is not host-gated (plugins/__init__.py registers it with a plain register(GaugeChartPlugin())), so the escape clause doesn't cover it. It's the only registered non-host-gated type missing from the tool that generates charts. An LLM asked for a gauge reads this docstring, sees no gauge, and declines or falls back to big_number — even though the schema/registry/union wiring all work if gauge_chart is passed.
Fix: add gauge to the one of: list (line ~90), a per-type entry in the ~127–135 block (chart_type='gauge_chart': required metric; optional groupby, min_val, max_val, row_limit), and a "gauge" / "dial" / "speedometer" -> chart_type='gauge_chart' line in the ~150 quick-lookup.
🟢 Low — get_chart_type_schema docstring contradicts its own code
This PR adds "gauge_chart": TypeAdapter(GaugeChartConfig) to _CHART_TYPE_ADAPTERS and a gauge example, but the function docstring (get_chart_type_schema.py:293-295) still says "Core types are xy, table, pie, … and waterfall" — no gauge. Severity limited because the impl's error path lists sorted(get_registry().all_types()), which does include gauge_chart. Fix: append gauge_chart to that sentence.
🟢 Low — multi-dial "top N" is unordered (pie opts in, gauge doesn't)
map_pie_config emits sort_by_metric (schema default True) so pie's row-limited output is deterministically the top N by metric; GaugeChartConfig/map_gauge_config emit no sort_by_metric, and gauge's buildQuery.ts only sets orderby when sort_by_metric is truthy. So a gauge with a groupby and more rows than row_limit (≤10) gets LIMIT with no ORDER BY — an arbitrary, run-to-run-unstable set of dials. Honest caveat: the shared sort_by_metric control has no default, so a UI-built gauge behaves the same way; this is a consistency gap with pie, not a regression. Consider adding sort_by_metric to the schema (default True) to make dial selection deterministic.
🟢 Low — test gaps on the non-obvious defaults
test_gauge_chart.py is otherwise thorough (10-cap, inverted-bounds, groupby-purity, saved-metric→string, union dispatch), but the one behavior with a non-obvious default — map_gauge_config defaulting color_scheme to "supersetColors" when None — has no assertion, nor does emission of the min_val/max_val keys. Fix: assert map_gauge_config(config)["color_scheme"] == "supersetColors" for a scheme-less config, plus an explicit-scheme passthrough.
What's solid (no changes needed): the form_data mapping is faithful to the frontend — every emitted key (groupby, metric, row_limit, min_val, max_val, color_scheme) is a real gauge control that buildQuery/transformProps consume; row_limit ge=1, le=10 matches the frontend's 1–10 dial cap exactly; min/max = None maps to the frontend's null auto-scale; to_form_data ignoring dataset_id, the color_scheme or "supersetColors" default, and the normalize_column_refs canonicalization all match PieChartPlugin. The two gauge-specific validators (reject_inverted_bounds, reject_metric_style_groupby) are more complete than the siblings' guards. None of the findings touch the security model.
Net: none of these block a single-dial gauge (the common case); the Medium is about discoverability, which is the PR's stated purpose, so worth fixing before merge.
aminghadersohi
left a comment
aminghadersohi
left a comment
There was a problem hiding this comment.
Round 3 follow-up at head 6e8ab393, focused on independently verifying @rebenitez1802's CHANGES_REQUESTED. I checked each point against the code at this head (not the review's snapshot). Net: the discoverability point is real, but one item is already fixed here and one is partly inaccurate.
🟡 Medium — generate_chart docstring omits gauge_chart → holds up (confirmed)
Verified the premise: gauge is not host-gated — plugins/__init__.py:52 registers it plainly and gauge.py has no is_enabled/feature-flag override (contrast interactive_pivot.py:131, which gates on feature_flag_manager). So the generate_chart docstring's escape clause ("plus host-gated types returned by get_chart_type_schema") genuinely does not cover gauge, and gauge is absent from its one of: list, its per-type block, and its natural-language quick-lookup. An LLM driving generate_chart won't discover it. Fair call, and it's the one worth acting on since discoverability is this PR's stated purpose. Note generate_chart.py is currently outside the PR's 6-file diff, so this is a small scope expansion rather than a fix to a changed line.
🟢 get_chart_type_schema docstring — already fixed at this head (finding is stale)
The claim that the docstring "still says … no gauge" no longer matches head 6e8ab393. The PR's latest commit ("docs(mcp): list gauge_chart in get_chart_type_schema docstring") changed exactly that line — the authoritative PR diff shows - table, pie, pivot_table, → + table, pie, gauge_chart, pivot_table,, and the docstring at head now reads "Core types are xy, table, pie, gauge_chart, pivot_table, …". So this one's done; no further change needed.
🟢 Unordered multi-dial top-N (sort_by_metric) — holds up, and it matches my round-2 note
This is the same consistency gap I flagged earlier: map_gauge_config emits no sort_by_metric/orderby, so a gauge with a groupby exceeding row_limit (≤10) gets LIMIT with no ORDER BY → an arbitrary dial set. As @rebenitez1802 fairly caveats, the shared sort_by_metric control has no default so a UI-built gauge behaves identically — it's a consistency gap with pie, not a regression. Optional hardening, not a blocker.
🟢 Test gaps — half confirmed
color_schemedefault (config.color_scheme or "supersetColors"): untested — confirmed, nocolor_scheme/supersetColorsassertion intest_gauge_chart.py. Worth a one-line assertion.min_val/max_valkey emission: already tested —test_gauge_form_data_with_dials_and_rangeassertsform_data["min_val"] == 0and["max_val"] == 100(test file lines ~130–131). So only thecolor_schemedefault is the real gap.
No new issues beyond the above in the 6-file diff, and the earlier is_metric/reject_inverted_bounds fixes remain intact (17 tests still pass locally). codeant's inverted-bounds thread is still open but its substance was addressed by reject_inverted_bounds — leaving resolution to the author. CI at 6e8ab393: 48 success / 2 neutral / 9 skipped, no failing or pending legs.
As before, I'm keeping this a comment (external-contributor PR). Recommendation to the author: the Medium (add gauge to generate_chart's docstring) plus the one-line color_scheme test are the only substantive to-dos; the get_chart_type_schema item is already handled.
Code Review Agent Run #578ec5Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
… (review) Addresses rebenitez1802's changes-requested review: - generate_chart.py docstring now lists gauge_chart in the one-of chart_type list, adds a per-type entry (required metric; optional groupby/min_val/ max_val), and a 'gauge'/'dial'/'speedometer' quick-lookup alias. gauge is not host-gated, so the 'plus host-gated types' clause didn't cover it — an LLM asked for a gauge previously saw no gauge and fell back to big_number. - Add sort_by_metric (default True) to GaugeChartConfig + emit it, and translate it to a query orderby in _build_single_query_dict, so a multi-dial gauge with more rows than row_limit keeps the top-N dials deterministically rather than an arbitrary set. - Tests: color_scheme default + min/max emission; sort_by_metric -> descending metric orderby in the built query. - (get_chart_type_schema docstring already lists gauge_chart from the rebase.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@rebenitez1802 — thanks for the thorough pass; fixed in
|
Code Review Agent Run #273da9Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
rebenitez1802
left a comment
rebenitez1802
left a comment
There was a problem hiding this comment.
Approve: all four points from my prior review are addressed, and the sort_by_metric fix is at the right layer. Two optional nits below, none blocking.
Re-reviewed the new commits. Confirmed resolved:
generate_chartdocstring now listsgauge_chartin theone of:types, adds a per-type entry (requiredmetric; optionalgroupby/min_val/max_val), and a"gauge" / "dial" / "speedometer"quick-lookup line.get_chart_type_schemadocstring core-types sentence now includesgauge_chart.- Deterministic multi-dial ordering —
GaugeChartConfig.sort_by_metric(defaultTrue) is emitted bymap_gauge_config, and the newchart_helpers.py_build_single_query_dicttranslates the flag intoorderby=[(metrics[0], False)]. Nice catch putting it there: the MCP path builds the query dict directly and never derivesorderbythe waybuildQuery.tsdoes, so this is the correct layer. The tuple/descending shape matches both the frontend contract (Pie/buildQuery.ts,Gauge/buildQuery.ts) and the existing deck path. - Tests now assert the
color_schemedefault,min_val/max_valemission, and thatsort_by_metricproduces anorderby.
🟢 Low — the shared orderby change also alters pie's MCP output, and that path is untested
_build_single_query_dict is shared, so this fix also newly adds orderby to pie MCP queries (pie has always emitted sort_by_metric=True, but the query dict previously ignored it). That's a correct frontend-alignment bugfix — a row_limit on an unordered result was dropping arbitrary rows instead of top-N — but it's a behavior change to a shipped chart type covered only by the new gauge test. Consider one assertion that map_pie_config → build_query_dicts_from_form_data yields orderby[0][1] is False, to lock the shared behavior for the pre-existing sibling it actually changes.
🟢 Low — comment lists chart types that don't apply in the MCP path
The chart_helpers.py comment says "pie/funnel/treemap/sankey/gauge," but only pie and gauge emit sort_by_metric among MCP-generatable types; funnel/treemap/sankey have no plugin/map that sets it (it'd only apply to a passed-through saved chart). Consider trimming to "pie/gauge (and any passed-through chart whose form_data sets sort_by_metric)."
🟢 Low — misleading positional arg in the new test
test_gauge_sort_by_metric_becomes_orderby calls build_query_dicts_from_form_data(form_data, 1, "table"); the third positional is datasource_type, not viz_type. Harmless (viz_type comes from form_data and resolve_datasource_engine is monkeypatched), but "table" in a gauge test reads as a copy-paste artifact.
Nice iteration — the query-layer fix in particular is better than a docstring-only edit would have been.
|
@gkneighb Thanks—the prior bounds, metric-shaped groupby, saved-query ordering, docs, and color-default fixes remain correct at
I ran the 19 focused Gauge tests, 1,386 chart MCP tests, and 3,712 broader MCP tests (one unrelated environment health-test failure) plus the native-state, numeric, ordering, preview, and casing reproductions. These gaps remain with green PR CI. Shared builder/error/canonicalization work from #43737 and omission-aware update work from #43679 should be reused/rebased rather than duplicated. I did not modify or push to your branch. |
3 participants
SUMMARY
Adds a
generate_chartMCP plugin for the gauge chart type (viz_type: gauge_chart), so the MCPgenerate_charttool can produce gauge charts. Gauge is a shipped Superset viz type that had no MCP plugin; this closes that gap.The plugin mirrors the frontend Gauge
buildQuerycontract: a singlemetricwhose value the dial displays, plus an optional multigroupby— with no groupby the chart is a single dial; with a groupby it renders one dial per row (capped at the frontend's limit of 10).min_val/max_valfix the dial scale (both default to auto).Follows the established plugin pattern (
pie/funnel/waterfall) — a config schema, amap_*_configmapper, a plugin class, and registration:GaugeChartConfig—metricrequired; optionalgroupby(list, one dial per row),row_limit(default 10, capped 1–10 to match the frontend),min_val/max_val,filters,color_scheme. Added to theChartConfigdiscriminated union and theget_chart_type_schemaadapters.groupbyentries may not besaved_metric/sql_expression(dimensions, not metrics).map_gauge_config— maps the config toform_data; saved metrics pass through as a bare name string, ad-hoc metrics as SIMPLE/SQL adhoc objects (via the sharedcreate_metric_object).GaugeChartPlugin— registered in the plugin registry;gauge_chartwas already present in the recommendation category map.The field set is intentionally minimal (core query contract + the dial scale). Cosmetic ECharts controls (start/end angle, intervals, pointer, progress, etc.) are left for a follow-up.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend/MCP only, no UI change.
TESTING INSTRUCTIONS
pytest tests/unit_tests/mcp_service/chart/test_gauge_chart.py— 17 unit tests cover schema validation (metric required, groupby optional, row-limit cap at 10, groupby-not-a-metric, extra-field rejection),ChartConfigunion dispatch,form_datamapping (viz_type/groupby/metric/row_limit/min_val/max_val/filters/saved-metric), and registry integration (registration,resolve_viz_type,display_name,pre_validatewith and without groupby).To exercise end-to-end: call the
generate_chartMCP tool with{"chart_type": "gauge_chart", "metric": {"name": "progress", "aggregate": "AVG"}}.ADDITIONAL INFORMATION
🤖 Generated with Claude Code