codeant-ai-for-open-source[bot] commented on code in PR #38388:
URL: https://github.com/apache/superset/pull/38388#discussion_r2882596204
##########
superset/mcp_service/sql_lab/schemas.py:
##########
@@ -113,6 +130,14 @@ class ExecuteSqlResponse(BaseModel):
statements: list[StatementInfo] | None = Field(
None, description="Per-statement execution info (for multi-statement
queries)"
)
+ multi_statement_warning: str | None = Field(
+ None,
+ description=(
+ "Warning when multiple data-bearing statements were executed. "
+ "The top-level rows/columns contain only the last statement's
results. "
+ "Check each entry in the statements array for per-statement data."
+ ),
+ )
Review Comment:
**Suggestion:** The `multi_statement_warning` field description claims
top-level rows/columns are from the last statement, but the implementation uses
the last data-bearing statement, so the schema documentation is inaccurate and
can cause client misunderstandings about response semantics. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ API clients may misinterpret which statement rows represent.
- ⚠️ LLM prompts guided by schema docs become less accurate.
- ⚠️ Confusing behavior for multi-statement queries ending with DDL/DML.
```
</details>
```suggestion
multi_statement_warning: str | None = Field(
None,
description=(
"Warning when multiple data-bearing statements were executed. "
"The top-level rows/columns contain only the last data-bearing
statement's results. "
"Check each entry in the statements array for per-statement
data."
),
)
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `superset/mcp_service/sql_lab/schemas.py` and locate
`ExecuteSqlResponse` at lines
111–140; observe `multi_statement_warning` description at lines 133–139
stating: "The
top-level rows/columns contain only the last statement's results."
2. Open `superset/mcp_service/sql_lab/execute_sql.py` and inspect
`_convert_to_response()`
(the function that builds `ExecuteSqlResponse`), where rows/columns are set
from the last
**data-bearing** statement (per PR description and unit tests behavior).
3. Execute a multi-statement SQL query through the MCP `execute_sql` tool
(entrypoint in
`superset/mcp_service/sql_lab/execute_sql.py`, e.g., `"SELECT 1 AS a; SET
statement_timeout = 1000"`), so the last statement is non data-bearing but
an earlier
statement returns rows.
4. Inspect the resulting `ExecuteSqlResponse`: `rows`/`columns` contain data
from the last
data-bearing statement (`SELECT 1`), contradicting the schema description
that they come
from the last statement, demonstrating that the field documentation is
inaccurate relative
to actual behavior.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/sql_lab/schemas.py
**Line:** 133:140
**Comment:**
*Logic Error: The `multi_statement_warning` field description claims
top-level rows/columns are from the last statement, but the implementation uses
the last data-bearing statement, so the schema documentation is inaccurate and
can cause client misunderstandings about response semantics.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38388&comment_hash=67a76eb1903b674eaa2b007f6a14786b4ebe1592d6889be73c410541457dbd3a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38388&comment_hash=67a76eb1903b674eaa2b007f6a14786b4ebe1592d6889be73c410541457dbd3a&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]
