codeant-ai-for-open-source[bot] commented on code in PR #37516:
URL: https://github.com/apache/superset/pull/37516#discussion_r3694622840
##########
superset/common/query_context_processor.py:
##########
@@ -228,6 +260,14 @@ def get_df_payload(
"label_map": label_map,
"warning": warning,
}
+ timing = QueryAcquisitionTiming(
+ cache_key_ns=cache_key_ns,
+ cache_read_ns=cache_read_ns,
+ source_ns=source_ns,
+ cache_write_ns=cache_write_ns,
+ cache_hit=cache_hit,
+ )
+ return QueryAcquisitionResult(payload=payload, timing=timing)
Review Comment:
**Suggestion:** The reported lifecycle total excludes label-map
construction, dataframe column normalization, warning generation, and payload
assembly because those operations occur after the acquisition timer stops and
before `_get_full_with_timing` starts materialization timing. Consequently,
`total_ms` and all phase totals under-report the actual server-side query
processing time. Move these operations into the materialization measurement or
include them in the acquisition timing. [logic error]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ `/api/v1/chart/data` timing under-reports server processing.
- โ ๏ธ `chart_data.query.total_ms` metrics omit payload preparation.
- โ ๏ธ Slow-query diagnosis can misattribute response latency.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=89fee5752d3a4e84af2c15f5933e7ffc&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=89fee5752d3a4e84af2c15f5933e7ffc&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/common/query_context_processor.py
**Line:** 263:270
**Comment:**
*Logic Error: The reported lifecycle total excludes label-map
construction, dataframe column normalization, warning generation, and payload
assembly because those operations occur after the acquisition timer stops and
before `_get_full_with_timing` starts materialization timing. Consequently,
`total_ms` and all phase totals under-report the actual server-side query
processing time. Move these operations into the materialization measurement or
include them in the acquisition timing.
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=ff19642d00ce1270119fb54282282a4f1c406bc8057489552988250b06d2303d&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=ff19642d00ce1270119fb54282282a4f1c406bc8057489552988250b06d2303d&reaction=dislike'>๐</a>
##########
superset/common/query_context_processor.py:
##########
@@ -113,8 +130,16 @@ def get_df_payload(
and len(query_obj.filter) > 0
):
cache.is_loaded = False
-
+ cache.is_cached = None
+ cache.cache_dttm = None
+ cache.cache_value = None
+ cache.queried_dttm = None
Review Comment:
**Suggestion:** The cache entry is marked as unloaded by clearing only
selected fields, but stale `status`, `stacktrace`, and BigQuery memory-limit
metadata remain. If validation then fails, the returned failed payload can
retain metadata and warning state from the previously cached successful result
instead of representing the new failure. Reset the complete cache-manager state
or create a fresh miss object before executing the query. [stale reference]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Failed chart queries can expose stale result metadata.
- โ ๏ธ BigQuery truncation warnings may describe old results.
- โ ๏ธ Query-only responses can retain the previous dataframe.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=aa6b60be25aa47a9b55bea923d930988&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=aa6b60be25aa47a9b55bea923d930988&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/common/query_context_processor.py
**Line:** 132:136
**Comment:**
*Stale Reference: The cache entry is marked as unloaded by clearing
only selected fields, but stale `status`, `stacktrace`, and BigQuery
memory-limit metadata remain. If validation then fails, the returned failed
payload can retain metadata and warning state from the previously cached
successful result instead of representing the new failure. Reset the complete
cache-manager state or create a fresh miss object before executing the query.
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=5117ab78d9ebacffd4d83fc1e259ff03b39e37056e29d65cf47fe69362f2ad4b&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=5117ab78d9ebacffd4d83fc1e259ff03b39e37056e29d65cf47fe69362f2ad4b&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]
