bito-code-review[bot] commented on code in PR #42144:
URL: https://github.com/apache/superset/pull/42144#discussion_r3616182245
##########
superset/mcp_service/dataset/schemas.py:
##########
@@ -807,6 +807,22 @@ class QueryDatasetFilter(BaseModel):
)
+# Bracket shorthands (e.g. "[year]", "[quarter]") are not a Superset
+# time-range grammar — they appear when an LLM copies a grain token from a
+# dashboard filter context. Map them to the equivalent "Last <unit>" form
+# that get_since_until() does support.
+_BRACKET_SHORTHAND_TO_TIME_RANGE: dict[str, str] = {
+ "[second]": "Last second",
+ "[minute]": "Last minute",
+ "[hour]": "Last hour",
Review Comment:
<!-- Bito Reply -->
The reviewer's concern regarding the reachability of 'Last hour' is
technically accurate based on the existing `get_since_until()` implementation
in `date_parser.py`. However, your approach of mapping these shorthands to
explicit `DATEADD` expressions in `schemas.py` effectively bypasses the
`date_parser.py` regex entirely, resolving the issue by ensuring both ends of
the time range are evaluated against `now`.
Since your implementation avoids the pre-existing bug in `get_since_until()`
and provides a robust workaround for the specific shorthand tokens, the
suggestion is addressed by your design choice. You do not need to apply the
reviewer's suggestion to modify `date_parser.py` as it is out of scope for this
PR.
**superset/mcp_service/dataset/schemas.py**
```
+_BRACKET_SHORTHAND_TO_TIME_RANGE: dict[str, str] = {
+ "[second]": "Last second",
+ "[minute]": "Last minute",
+ "[hour]": "Last hour",
```
--
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]
