codeant-ai-for-open-source[bot] commented on code in PR #37516:
URL: https://github.com/apache/superset/pull/37516#discussion_r3557436394
##########
superset/views/api.py:
##########
@@ -74,6 +75,11 @@ def query(self) -> FlaskResponse:
query_context.raise_for_access()
result = query_context.get_payload()
payload_json = result["queries"]
+ # Strip the internal timing sentinel (and surface public timing when
+ # CHART_DATA_INCLUDE_TIMING is enabled); get_payload() always injects
+ # it, but only the chart-data path finalizes it otherwise.
+ for query in payload_json:
Review Comment:
**Suggestion:** The new comment states that `get_payload()` “always injects”
the internal timing sentinel, but that is not true for result types like
query-only/metadata responses that do not go through dataframe payload
generation. This is a factual comment/code mismatch that can mislead future
maintainers into assuming timing state always exists; update the comment to
describe that timing is present only for payloads produced via dataframe
execution paths. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Comment misleads about timing for metadata-only result types.
- ⚠️ Future refactors may wrongly assume timing exists universally.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Inspect the legacy chart query endpoint in `superset/views/api.py` around
lines 65–84;
the `query()` method calls `query_context.get_payload()` and includes the
comment:
`get_payload() always injects` the internal timing sentinel (lines 19–21
from the file
read, corresponding to diff lines 79–81).
2. Follow `QueryContext.get_payload()` in `superset/common/query_context.py`
lines 58–64,
which delegates to `QueryContextProcessor.get_payload(cache_query_context,
force_cached)`
(processor defined in `superset/common/query_context_processor.py`).
3. Examine `QueryContextProcessor.get_payload()` in
`superset/common/query_context_processor.py` lines 108–146: it calls
`get_query_results(...)` from `superset/common/query_actions.py` with
`result_type` taken
from `query_obj.result_type or self._query_context.result_type`.
4. In `superset/common/query_actions.py`, see `_result_type_functions` and
helpers: for
`ChartDataResultType.QUERY` (lines 79–98), `_get_query()` builds a payload
containing
`language`, `query`, and optional `error` but never adds `TIMING_KEY`; only
`_get_full()`
(lines 154–206) calls `query_context.get_df_payload()`, whose return value
includes
`TIMING_KEY` (see `TIMING_KEY: timing` at
`superset/common/query_context_processor.py:249). Thus, for result types
like `QUERY`,
`COLUMNS`, or `TIMEGRAINS`, `get_payload()` returns entries in
`result["queries"]` without
any timing sentinel, contradicting the comment that `get_payload() always
injects` it.
`finalize_timing_payload()` (in `superset/common/chart_data_timing.py` lines
22–26) safely
no-ops when `TIMING_KEY` is absent, but the comment remains factually
inaccurate.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7427bb1e2cab4f17b433154408ba35da&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=7427bb1e2cab4f17b433154408ba35da&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/views/api.py
**Line:** 79:81
**Comment:**
*Comment Mismatch: The new comment states that `get_payload()` “always
injects” the internal timing sentinel, but that is not true for result types
like query-only/metadata responses that do not go through dataframe payload
generation. This is a factual comment/code mismatch that can mislead future
maintainers into assuming timing state always exists; update the comment to
describe that timing is present only for payloads produced via dataframe
execution paths.
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%2F37516&comment_hash=a0f1066e31526e2885fe3eabfda143212821f8f1c5e7283c5ca4cfb1e5d0860f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=a0f1066e31526e2885fe3eabfda143212821f8f1c5e7283c5ca4cfb1e5d0860f&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]
