mikebridge opened a new pull request, #45153:
URL: https://github.com/apache/superset/pull/45153
### SUMMARY
The MCP tools `get_compatible_metrics` and `get_compatible_dimensions`
silently dropped selected names they did not recognize when the source was an
external semantic view. `SemanticView.get_compatible_*` translates names to
semantic-layer objects and skips any name it can't find. A typo, a display
label or a provider ID therefore produced a successful answer for a smaller
selection than the caller asked for, with no sign that anything was ignored.
The SQL-dataset path of the same tools already rejected unknown names, and so
does `get_table` for filters.
On the external-view path, both tools now validate `selected_metrics` and
`selected_dimensions` against the view's metric and dimension names. The check
runs after the access check and before the provider's compatibility call.
Unknown names return a `ValidationError` that names each one, using the
existing shared validator. For example:
`Unknown metric: 'bogus_metric'; Unknown dimension: 'bogus_dimension'`
Close-match suggestions come only from the view's member names. Provider
metric or dimension definitions never appear in the error. The validator is a
small shared helper in `mcp_service/utils/query_utils.py`, used by both tools.
Behavior:
| Selection on an external semantic view | Before | After |
| --- | --- | --- |
| All names known | Compatible members returned | Unchanged |
| Empty selection | Compatible members returned | Unchanged |
| Contains an unknown metric or dimension name | Success, computed as if the
unknown name were absent | `ValidationError` naming the unknown fields; no
provider compatibility call |
The source-selection checks (`dataset_id` versus `view_id`) were merged into
one branch, keeping both existing error messages, and tests now pin those
messages. The MCP server docs describe the validation.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable: MCP tool response change, with no UI. The table above
describes the behavior before and after.
### TESTING INSTRUCTIONS
```bash
pytest
tests/unit_tests/mcp_service/semantic_layer/tool/test_compatibility_selection_validation.py
\
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_metrics.py
\
tests/unit_tests/mcp_service/semantic_layer/tool/test_get_compatible_dimensions.py
```
The new test calls both tools through the MCP client against a real
`SemanticView`. The view is backed by core `Metric`/`Dimension` objects whose
IDs differ from their names (`orders.bookings` / `bookings`), so it exercises
the model's real name translation.
- **Invalid cases:** an unknown metric, an unknown dimension, or both. Each
returns the fixed `ValidationError`, never calls the provider's compatibility
method, and leaks no provider definitions.
- **Valid and empty cases:** the provider still receives the same object
sets.
- **Before the fix:** the six invalid cases return success, and the four
valid or empty cases pass on both.
Local validation:
- The MCP semantic-layer tool tests pass. The one exception is an existing
timezone-dependent `get_table` test that this change does not touch.
- Changed-file pre-commit passed, including mypy, ruff and pylint.
- No live semantic provider was used.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
@aminghadersohi
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
