codeant-ai-for-open-source[bot] commented on code in PR #42153:
URL: https://github.com/apache/superset/pull/42153#discussion_r3692610383
##########
superset/utils/webdriver.py:
##########
@@ -482,16 +482,31 @@ def get_screenshot( # pylint: disable=too-many-locals,
too-many-statements # n
logger.exception("Timed out requesting url %s", url)
raise
+ slice_container_elems: list[Locator] = []
+ rendered_chart_count = 0
try:
# chart containers didn't render
logger.debug("Wait for chart containers to draw at url:
%s", url)
slice_container_locator = page.locator(".chart-container")
- for slice_container_elem in slice_container_locator.all():
+ # One-time snapshot: containers mounting after this point
+ # are neither waited on nor counted, so the progress
+ # numbers below describe the snapshot, not the final DOM.
+ slice_container_elems = slice_container_locator.all()
+ for slice_container_elem in slice_container_elems:
slice_container_elem.wait_for()
+ rendered_chart_count += 1
Review Comment:
**Suggestion:** `Locator.wait_for()` only waits for the locator's default
presence/visibility condition; it does not establish that a chart has rendered
or reached a terminal state. The counter is therefore incremented for
containers that may still contain a spinner or no chart content, and the
timeout log can falsely report them as rendered. Use the actual chart readiness
predicate for the progress count, or describe the count as containers located
rather than rendered. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Timeout logs can overstate rendered chart progress.
- โ ๏ธ Report diagnostics may mislead operators during slow loads.
- โ
Standard and tiled readiness gates remain unaffected.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d83f4f984a9c470694d1bc305e9bace5&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=d83f4f984a9c470694d1bc305e9bace5&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/webdriver.py
**Line:** 496:497
**Comment:**
*Comment Mismatch: `Locator.wait_for()` only waits for the locator's
default presence/visibility condition; it does not establish that a chart has
rendered or reached a terminal state. The counter is therefore incremented for
containers that may still contain a spinner or no chart content, and the
timeout log can falsely report them as rendered. Use the actual chart readiness
predicate for the progress count, or describe the count as containers located
rather than rendered.
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%2F42153&comment_hash=1703803fcfe063a794b5628054e993ce5d3682d4923505b89228afbf44aba470&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42153&comment_hash=1703803fcfe063a794b5628054e993ce5d3682d4923505b89228afbf44aba470&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]