codeant-ai-for-open-source[bot] commented on code in PR #43225:
URL: https://github.com/apache/superset/pull/43225#discussion_r3789645869
##########
superset/charts/schemas.py:
##########
@@ -528,17 +526,15 @@ class
ChartDataRollingOptionsSchema(ChartDataPostProcessingOperationOptionsSchem
Rolling operation config.
"""
- columns = (
- fields.Dict(
- metadata={
- "description": "columns on which to perform rolling, mapping
source "
- "column to target column. For instance, `{'y': 'y'}` will
replace the "
- "column `y` with the rolling value in `y`, while `{'y': 'y2'}`
will add " # noqa: E501
- "a column `y2` based on rolling values calculated from `y`,
leaving the " # noqa: E501
- "original column `y` unchanged.",
- "example": {"weekly_rolling_sales": "sales"},
- },
- ),
+ columns = fields.Dict(
+ metadata={
+ "description": "columns on which to perform rolling, mapping
source "
+ "column to target column. For instance, `{'y': 'y'}` will replace
the "
+ "column `y` with the rolling value in `y`, while `{'y': 'y2'}`
will add "
+ "a column `y2` based on rolling values calculated from `y`,
leaving the "
+ "original column `y` unchanged.",
+ "example": {"weekly_rolling_sales": "sales"},
+ },
)
Review Comment:
**Suggestion:** The restored `Dict` field accepts arbitrary value types,
although the rolling implementation requires a mapping from source column names
to string target column names and later passes those values to
`_append_columns`. A schema consumer can therefore accept values such as lists
or numbers and only fail during pandas processing. Declare string key and value
fields so malformed rolling mappings are rejected or accurately documented at
the schema boundary. [type error]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ OpenAPI permits malformed rolling column mappings.
- โ Invalid mappings can fail during rolling post-processing.
- โ ๏ธ Frontend normally sends string-to-string mappings at
`rollingWindowOperator.ts:48`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a6ff2939f2584808b7af21193f4aa7b3&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=a6ff2939f2584808b7af21193f4aa7b3&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/charts/schemas.py
**Line:** 529:538
**Comment:**
*Type Error: The restored `Dict` field accepts arbitrary value types,
although the rolling implementation requires a mapping from source column names
to string target column names and later passes those values to
`_append_columns`. A schema consumer can therefore accept values such as lists
or numbers and only fail during pandas processing. Declare string key and value
fields so malformed rolling mappings are rejected or accurately documented at
the schema boundary.
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%2F43225&comment_hash=203581c5c280148841a81e29d7a708b2f6eeba349cc657ba9ca090c55d048cbb&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43225&comment_hash=203581c5c280148841a81e29d7a708b2f6eeba349cc657ba9ca090c55d048cbb&reaction=dislike'>๐</a>
##########
superset/charts/schemas.py:
##########
@@ -858,15 +854,13 @@ class
ChartDataPivotOptionsSchema(ChartDataPostProcessingOperationOptionsSchema)
Pivot operation config.
"""
- index = (
- fields.List(
- fields.String(allow_none=False),
- metadata={
- "description": "Columns to group by on the table index
(=rows)",
- "minLength": 1,
- },
- required=True,
- ),
+ index = fields.List(
+ fields.String(allow_none=False),
+ metadata={
+ "description": "Columns to group by on the table index (=rows)",
+ "minLength": 1,
+ },
+ required=True,
)
Review Comment:
**Suggestion:** `minLength` is only metadata and does not validate the
length of a Marshmallow `List`; moreover, the OpenAPI constraint for an array
is `minItems`, not `minLength`. Consequently, `index=[]` passes this restored
schema even though `pivot()` explicitly rejects an empty index. Use a length
validator and/or emit the correct array constraint so invalid pivot options are
rejected or accurately documented. [api mismatch]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Pivot schema accepts an operation-invalid empty index.
- โ Direct pivot requests fail during post-processing.
- โ ๏ธ OpenAPI advertises the wrong array constraint.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1eeeb1fed7474bd0be84f7aed45c0765&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=1eeeb1fed7474bd0be84f7aed45c0765&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/charts/schemas.py
**Line:** 857:864
**Comment:**
*Api Mismatch: `minLength` is only metadata and does not validate the
length of a Marshmallow `List`; moreover, the OpenAPI constraint for an array
is `minItems`, not `minLength`. Consequently, `index=[]` passes this restored
schema even though `pivot()` explicitly rejects an empty index. Use a length
validator and/or emit the correct array constraint so invalid pivot options are
rejected or accurately documented.
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%2F43225&comment_hash=d10a22ea765f1cb413a40c42e6ef9b8d35212b2ca6325baeeaaac0d66349b5d4&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43225&comment_hash=d10a22ea765f1cb413a40c42e6ef9b8d35212b2ca6325baeeaaac0d66349b5d4&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]
