codeant-ai-for-open-source[bot] commented on code in PR #41611:
URL: https://github.com/apache/superset/pull/41611#discussion_r3533509152
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -2246,10 +2246,20 @@ class DataColumn(BaseModel):
display_name: str = Field(..., description="Human-readable column name")
data_type: str = Field(..., description="Inferred data type")
sample_values: List[Any] = Field(description="Representative sample
values")
- null_count: int = Field(description="Number of null values")
- unique_count: int = Field(description="Number of unique values")
+ null_count: int = Field(
+ description="Number of null values. Approximate — see
'statistics.sampled_rows'"
+ " if the source result set was larger than the row cap used to compute
it."
+ )
+ unique_count: int = Field(
+ description="Number of unique values. Approximate — see "
+ "'statistics.sampled_rows' if the source result set was larger than
the "
+ "row cap used to compute it."
+ )
statistics: Dict[str, Any] | None = Field(
- None, description="Column statistics if numeric"
+ None,
+ description="Column statistics if numeric. May include 'sampled_rows' "
+ "when null_count/unique_count were computed on a row-capped sample "
+ "rather than the full result set.",
)
Review Comment:
**Suggestion:** The schema description says `statistics` is only present for
numeric columns, but the new formatter now attaches `statistics.sampled_rows`
for any sampled column type. This mismatch will mislead clients and generated
docs; either remove the numeric-only claim or restrict `statistics` population
to numeric columns. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ MCP `get_table` metadata misdocuments `statistics` for non-numeric
columns.
- ⚠️ Documentation-generated clients may mis-handle `statistics` field
presence.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Inspect the DataColumn schema in
`superset/mcp_service/chart/schemas.py:2242-2259`,
where the `statistics` field is declared as `Dict[str, Any] | None =
Field(None,
description="Column statistics if numeric. May include 'sampled_rows' when
null_count/unique_count were computed on a row-capped sample rather than the
full result
set.")`.
2. Inspect the GetTableResponse schema in
`superset/mcp_service/semantic_layer/schemas.py:219-223`, which exposes
`columns:
list[DataColumn]` as part of the MCP `get_table` response contract used by
semantic-layer
clients.
3. In the get_table tool implementation, at
`superset/mcp_service/semantic_layer/tool/get_table.py:269`, observe the call
`columns_meta = format_data_columns(data, raw_columns)` that builds the
`DataColumn`
instances returned in `GetTableResponse.columns`.
4. In `superset/mcp_service/utils/response_utils.py:172-219`, see that
`format_data_columns` unconditionally sets `statistics={"sampled_rows":
len(stats_rows)}`
whenever `is_sampled` is true, regardless of `data_type`, so calling
`get_table` with a
large string or boolean column causes `statistics.sampled_rows` to be
present even though
the schema description says `statistics` is only for numeric columns,
creating a
documented-vs-actual behavior mismatch.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=30e04ad85f6a4817bfed1c77b0e8de20&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=30e04ad85f6a4817bfed1c77b0e8de20&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/mcp_service/chart/schemas.py
**Line:** 2258:2263
**Comment:**
*Comment Mismatch: The schema description says `statistics` is only
present for numeric columns, but the new formatter now attaches
`statistics.sampled_rows` for any sampled column type. This mismatch will
mislead clients and generated docs; either remove the numeric-only claim or
restrict `statistics` population to numeric columns.
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%2F41611&comment_hash=85dc49c5bf776b252bfd35260a8e09c079af26221acf5f217a047ce77cf42a22&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41611&comment_hash=85dc49c5bf776b252bfd35260a8e09c079af26221acf5f217a047ce77cf42a22&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]
