aminghadersohi opened a new pull request, #42283: URL: https://github.com/apache/superset/pull/42283
### SUMMARY MCP tools that accept a free-form `time_range` string (`query_dataset`, `get_table`, `manage_native_filters`) forward it straight into a `TEMPORAL_RANGE` filter or native filter default with no validation. `superset.utils.date_parser.get_since_until()` only rewrites a separator-less `time_range` into a bounded range when it recognizes one of a handful of prefixes (`Last`, `Next`, `previous calendar ...`, `Current ...`, `first ... of ...`). Anything else — `banana`, `this month`, lowercase `last week`, `[decade]` — silently falls through to an unbounded `(None, today)` result: no error, no warning, and the query returns the **entire table** with `success: true`. Verified live against a local Superset MCP server (Postgres metadata DB, `examples` data, dataset 28 `cleaned_sales_data`, 2823 rows total): ``` get_table(dataset_id=28, metrics=["count"], time_range="banana") -> 2823 rows, success: true, warnings: [] get_table(dataset_id=28, metrics=["count"], time_range="[year]") -> 2823 rows get_table(dataset_id=28, metrics=["count"], time_range="Last year") -> 0 rows (correct) get_table(dataset_id=28, metrics=["count"], time_range="2003-01-01 : 2004-01-01") -> 1000 rows (correct) ``` This PR adds a shared validator (`superset.mcp_service.common.time_range_validation`) that: - Normalizes bracket shorthands (`[second]` through `[year]`) to a form `get_since_until()` resolves correctly, following the pattern from #42144 (which scoped this normalization to `query_dataset` only). - Rejects any other separator-less value that `get_since_until()` would silently discard, with a `ValidationError` listing the accepted forms so an LLM caller can self-correct instead of receiving a full-table result with no error signal. Wired into `QueryDatasetRequest.time_range`, `GetTableRequest.time_range`, `FilterTimeSpec.default_time_range`, and `NativeFilterUpdateSpec.default_time_range` — closing the gap left open by #42144, which explicitly scoped `get_table` and `manage_native_filters` out (see [discussion](https://github.com/apache/superset/pull/42144#issuecomment-5037809658)). **Why the guard lives in the MCP schema layer, not in `get_since_until()` itself:** that function is also called from `jinja_context.py` and `semantic_layers/mapper.py` across the whole chart/dashboard query path. Making it raise there is a much larger blast radius and a separate discussion about whether silently-ignored time ranges are load-bearing anywhere. The MCP tools are the surface that accepts free-form, model-generated strings. ### BREAKING CHANGE Values like `this month` or lowercase `last week` that previously silently returned an unfiltered, full-table result now raise a `ValidationError`. This is the intended fix — callers previously had no signal that their filter was ignored, which is worse than a loud error for an LLM caller that could otherwise self-correct. ### TESTING INSTRUCTIONS - `pytest tests/unit_tests/mcp_service/common/test_time_range_validation.py` - `pytest tests/unit_tests/mcp_service/dataset/tool/test_query_dataset.py` - `pytest tests/unit_tests/mcp_service/semantic_layer/tool/test_get_table.py` - `pytest tests/unit_tests/mcp_service/dashboard/tool/test_manage_native_filters.py` - `pytest tests/unit_tests/utils/date_parser_tests.py` — confirms the shared validator's accepted-prefix grammar doesn't regress the existing `get_since_until()` test suite (this PR does not modify `date_parser.py`) - Manually: call `get_table` / `query_dataset` with `time_range` set to `banana`, `this month`, `last week` (lowercase), and `[decade]` against a dataset with a temporal column, and confirm each now returns a `ValidationError` instead of an unfiltered result. ### 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 -- 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]
