codeant-ai-for-open-source[bot] commented on code in PR #41527:
URL: https://github.com/apache/superset/pull/41527#discussion_r3503497725
##########
superset/utils/date_parser.py:
##########
@@ -426,10 +426,27 @@ def get_since_until( # pylint:
disable=too-many-arguments,too-many-locals,too-m
return None, None
if time_range and time_range.startswith("Last") and separator not in
time_range:
- time_range = time_range + separator + _relative_end
+ # For granular time units (second/minute/hour), "today" (midnight)
would be
+ # earlier than the computed since time (e.g. now-1h), causing a
+ # "From date cannot be larger than to date" error. Always use "now" for
+ # sub-day units so that the until bound is consistent with the since
bound.
+ _granular_last_pattern = (
+ r"^Last\s+[0-9]+\s+(second|minute|hour)s?$"
+ )
Review Comment:
**Suggestion:** The granular detection regex requires a numeric count
(`[0-9]+`), but the parser also accepts forms without numbers (e.g. `Last hour`
/ `Next hour`). Those valid forms bypass the new granular-now handling, causing
inconsistent behavior and reintroducing invalid or misleading ranges; make the
number optional in these granular patterns. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Time range API misparses no-number granular expressions.
- ❌ Charts using “Last hour” can raise from>to errors.
- ⚠️ Inconsistent behavior between “Last 1 hour” and “Last hour”.
- ⚠️ User-provided granular ranges may yield misleading windows.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset/views/api.py:110-22`, the `time_range()` API method accepts a
client-provided `timeRange` string (via rison payload) and passes it
directly to
`get_since_until(time_range=time_range["timeRange"],
time_shift=time_range.get("shift"))`,
so values like `"Last hour"` or `"Next hour"` are valid inputs.
2. In `superset/utils/date_parser.py:428-40`, when `time_range` is `"Last
hour"` and
contains no `" : "` separator, the `if time_range and
time_range.startswith("Last")`
branch runs. `_granular_last_pattern` on lines 433-435 requires a numeric
count `[0-9]+`,
so `"Last hour"` does not match; the else branch appends `_relative_end`
(typically
`"today"` from configuration), producing `"Last hour : today"`.
3. Later in the same function, the `time_range_lookup` list at
`superset/utils/date_parser.py:166-171` includes the pattern
`r"^(this|last|next|prior)\s{1,5}([0-9]+)?\s{0,5}(second|minute|hour|day|week|month|quarter|year)s?$"`,
where the number is optional. `"Last hour"` matches this pattern with
`scope='last'`,
`delta=None`, and `unit='hour'`, and `handle_scope_and_unit()` uses
`get_relative_base(unit, relative_start)` (which returns `'now'` for `hour`)
to compute a
`since` datetime of `'now - 1 hour'`.
4. The `until` datetime for `"Last hour"` comes from the second partition
`"today"` via
the fallback block at `superset/utils/date_parser.py:205-208`, yielding
midnight of the
current day (`2016‑11‑07 00:00:00` under `mock_parse_human_datetime`). For
typical daytime
values of `'now'` (e.g. `2016‑11‑07 09:30:10`), `since` (`08:30:10`) is
later than `until`
(`00:00:00`), so when chart code in `superset/viz.py:15-32` computes
`from_dttm` and
`to_dttm` and checks `if from_dttm and to_dttm and from_dttm > to_dttm`, it
raises
`QueryObjectValidationError(_("From date cannot be larger than to date"))`,
causing charts
or API calls that use `"Last hour"` to fail even though the expression is
accepted by the
parser.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4cc41186bb6449faac9c73ce75601a01&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4cc41186bb6449faac9c73ce75601a01&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/date_parser.py
**Line:** 433:435
**Comment:**
*Logic Error: The granular detection regex requires a numeric count
(`[0-9]+`), but the parser also accepts forms without numbers (e.g. `Last hour`
/ `Next hour`). Those valid forms bypass the new granular-now handling, causing
inconsistent behavior and reintroducing invalid or misleading ranges; make the
number optional in these granular patterns.
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
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41527&comment_hash=0f2d4d9d727511bfd5342ddfe52cf6545502233aed700efa77174924ae3299a5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41527&comment_hash=0f2d4d9d727511bfd5342ddfe52cf6545502233aed700efa77174924ae3299a5&reaction=dislike'>👎</a>
--
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]
