codeant-ai-for-open-source[bot] commented on code in PR #42357:
URL: https://github.com/apache/superset/pull/42357#discussion_r3639564830
##########
tests/unit_tests/common/test_query_context_processor.py:
##########
@@ -2151,3 +2151,37 @@ def
test_raise_for_access_evaluates_access_before_validate():
processor.raise_for_access()
query.validate.assert_not_called()
+
+
+def test_relative_offset_preserves_inner_bounds():
+ """
+ Regression test for #40501: Relative time comparison offset should
+ preserve inner bounds as the original (unshifted) period, not the shifted
one.
+
+ When comparing 2026-05-01 : 2026-05-28 with offset "365 days ago":
+ - inner_from_dttm should be 2026-05-01 (original), NOT 2025-05-01 (shifted)
+ - inner_to_dttm should be 2026-05-28 (original), NOT 2025-05-28 (shifted)
+ """
+ outer_from = datetime(2026, 5, 1)
+ outer_to = datetime(2026, 5, 28)
+ offset = "365 days ago"
+
+ shifted_from = get_past_or_future(offset, outer_from)
+ shifted_to = get_past_or_future(offset, outer_to)
+
+ # Verify the shifted dates are correct
+ assert shifted_from == datetime(2025, 5, 1)
+ assert shifted_to == datetime(2025, 5, 28)
+
+ # The fix: inner bounds should use original period, not shifted
+ # This ensures partial periods (e.g., May 1-28) compare against
+ # the equivalent partial period in the past, not the full bucket
+ inner_from_dttm = outer_from # Should be original, not shifted_from
+ inner_to_dttm = outer_to # Should be original, not shifted_to
+
+ assert inner_from_dttm == datetime(2026, 5, 1)
+ assert inner_to_dttm == datetime(2026, 5, 28)
+
+ # Verify inner bounds are NOT the shifted values
+ assert inner_from_dttm != shifted_from
+ assert inner_to_dttm != shifted_to
Review Comment:
**Suggestion:** This regression test does not exercise the actual query
context processing logic, so it will pass even if the production bug remains
unfixed. It only assigns local variables from `outer_from`/`outer_to` and
asserts those values, which is tautological. Build a real
`QueryObject`/processor flow (or call the method that applies the offset) and
assert `inner_from_dttm`/`inner_to_dttm` on the resulting object. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Relative offset comparison regression unprotected by effective unit
test.
- ⚠️ Future partial-period offset bugs may ship undetected.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open tests/unit_tests/common/test_query_context_processor.py and locate
test_relative_offset_preserves_inner_bounds defined at lines 2156–2187;
observe that it
only computes shifted_from/shifted_to, assigns inner_from_dttm = outer_from
and
inner_to_dttm = outer_to (lines 2179–2180), and asserts on these locals
(lines 2182–2187)
without calling any function from superset.common.query_context_processor.
2. Run pytest for this single test, e.g. pytest
tests/unit_tests/common/test_query_context_processor.py::test_relative_offset_preserves_inner_bounds;
note that the test passes because the assertions simply restate how the
local variables
were assigned.
3. In the production module superset/common/query_context_processor.py
(module referenced
by string
"superset.common.query_context_processor.security_manager.raise_for_access" at
line 2147 of the same test file), deliberately reintroduce the original bug
described in
the PR by setting inner_from_dttm and inner_to_dttm to the shifted
from_dttm/to_dttm
values inside the relative offset handling logic.
4. Re-run pytest for
tests/unit_tests/common/test_query_context_processor.py::test_relative_offset_preserves_inner_bounds
and observe that the test still passes even though the production logic is
now incorrect
again, demonstrating that this regression test does not exercise the actual
query context
processing path and therefore cannot catch regressions in the
relative-offset inner bound
behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8223251f2780427b97ef0965e6268ce9&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=8223251f2780427b97ef0965e6268ce9&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:** tests/unit_tests/common/test_query_context_processor.py
**Line:** 2179:2187
**Comment:**
*Logic Error: This regression test does not exercise the actual query
context processing logic, so it will pass even if the production bug remains
unfixed. It only assigns local variables from `outer_from`/`outer_to` and
asserts those values, which is tautological. Build a real
`QueryObject`/processor flow (or call the method that applies the offset) and
assert `inner_from_dttm`/`inner_to_dttm` on the resulting object.
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%2F42357&comment_hash=cebe128507134a1a623b7a60457c585ba46502f06e367b0e1edb6a14b5320a65&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42357&comment_hash=cebe128507134a1a623b7a60457c585ba46502f06e367b0e1edb6a14b5320a65&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]
