codeant-ai-for-open-source[bot] commented on code in PR #40775:
URL: https://github.com/apache/superset/pull/40775#discussion_r3404243746
##########
tests/unit_tests/mcp_service/chart/test_big_number_chart.py:
##########
@@ -177,6 +177,16 @@ def test_compare_lag_requires_trendline(self) -> None:
compare_lag=1,
)
+ def test_aggregation_requires_trendline(self) -> None:
Review Comment:
**Suggestion:** Add an inline docstring to this newly added test method to
satisfy the documentation rule for new functions. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added test method in the updated hunk, and it has no
docstring immediately under the function definition. The custom rule requires
newly added Python functions and classes to include docstrings, so the
suggestion correctly identifies a real violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9a5259fc79f94449aeed699985c31b5b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9a5259fc79f94449aeed699985c31b5b&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:** tests/unit_tests/mcp_service/chart/test_big_number_chart.py
**Line:** 180:180
**Comment:**
*Custom Rule: Add an inline docstring to this newly added test method
to satisfy the documentation rule for new functions.
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%2F40775&comment_hash=89fd410ac1bd9a2d2590e0294695a54f758e2e3c599e9a7e533a0cd0d19ef86d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40775&comment_hash=89fd410ac1bd9a2d2590e0294695a54f758e2e3c599e9a7e533a0cd0d19ef86d&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/test_big_number_chart.py:
##########
@@ -188,6 +198,33 @@ def test_with_filters(self) -> None:
assert config.filters is not None
assert len(config.filters) == 1
+ def test_with_aggregation_sum(self) -> None:
Review Comment:
**Suggestion:** Add a short docstring under this new test function
definition describing the behavior being validated. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This function was newly introduced in the PR and does not include a
docstring. Since the rule explicitly requires newly added Python functions to
be documented inline, this is a valid violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d05fc883df1345c789e4e52f2f6d803d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d05fc883df1345c789e4e52f2f6d803d&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:** tests/unit_tests/mcp_service/chart/test_big_number_chart.py
**Line:** 201:201
**Comment:**
*Custom Rule: Add a short docstring under this new test function
definition describing the behavior being validated.
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%2F40775&comment_hash=c3b7aa71bfc7d11f83549f5a0b1d15e71ffdb47d297d1c3f93e8259d69ab76e5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40775&comment_hash=c3b7aa71bfc7d11f83549f5a0b1d15e71ffdb47d297d1c3f93e8259d69ab76e5&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/test_big_number_chart.py:
##########
@@ -188,6 +198,33 @@ def test_with_filters(self) -> None:
assert config.filters is not None
assert len(config.filters) == 1
+ def test_with_aggregation_sum(self) -> None:
+ config = BigNumberChartConfig(
+ chart_type="big_number",
+ metric=ColumnRef(name="revenue", aggregate="SUM"),
+ temporal_column="ds",
+ show_trendline=True,
+ aggregation="sum",
+ )
+ assert config.aggregation == "sum"
+
+ def test_with_aggregation_last_value(self) -> None:
Review Comment:
**Suggestion:** Add a docstring for this newly introduced test function so
new code is documented inline. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a new test function added in the diff, and there is no docstring
beneath it in the final file state. That matches the documented-inline
requirement for new Python functions.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eb417cc63b7f4560b61f4a0d81c786ae&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=eb417cc63b7f4560b61f4a0d81c786ae&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:** tests/unit_tests/mcp_service/chart/test_big_number_chart.py
**Line:** 211:211
**Comment:**
*Custom Rule: Add a docstring for this newly introduced test function
so new code is documented inline.
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%2F40775&comment_hash=5d483c26ebf06d61411dcd3434edfdf7f999b398aee38cb237e7c179bce2d8bc&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40775&comment_hash=5d483c26ebf06d61411dcd3434edfdf7f999b398aee38cb237e7c179bce2d8bc&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/test_big_number_chart.py:
##########
@@ -188,6 +198,33 @@ def test_with_filters(self) -> None:
assert config.filters is not None
assert len(config.filters) == 1
+ def test_with_aggregation_sum(self) -> None:
+ config = BigNumberChartConfig(
+ chart_type="big_number",
+ metric=ColumnRef(name="revenue", aggregate="SUM"),
+ temporal_column="ds",
+ show_trendline=True,
+ aggregation="sum",
+ )
+ assert config.aggregation == "sum"
+
+ def test_with_aggregation_last_value(self) -> None:
+ config = BigNumberChartConfig(
+ chart_type="big_number",
+ metric=ColumnRef(name="revenue", aggregate="SUM"),
+ temporal_column="ds",
+ show_trendline=True,
+ aggregation="LAST_VALUE",
+ )
+ assert config.aggregation == "LAST_VALUE"
+
+ def test_aggregation_defaults_to_none(self) -> None:
Review Comment:
**Suggestion:** Add a docstring to this new test method to comply with the
requirement that newly added functions be documented. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added method has no docstring in the final code. The custom rule
for new Python functions/classes without docstrings applies here, so the
suggestion is justified.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7fe3b9d630fe4b11b48d0dd0b6cf7794&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=7fe3b9d630fe4b11b48d0dd0b6cf7794&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:** tests/unit_tests/mcp_service/chart/test_big_number_chart.py
**Line:** 221:221
**Comment:**
*Custom Rule: Add a docstring to this new test method to comply with
the requirement that newly added functions be 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%2F40775&comment_hash=e9374a4c49fcb9208e36548c53943bb9737b1f2be0bf7411f60b0276373cbf41&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40775&comment_hash=e9374a4c49fcb9208e36548c53943bb9737b1f2be0bf7411f60b0276373cbf41&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/chart/test_big_number_chart.py:
##########
@@ -307,6 +344,48 @@ def test_no_trendline_fields_for_total(self) -> None:
assert "time_grain_sqla" not in form_data
assert "start_y_axis_at_zero" not in form_data
+ def test_with_aggregation_sum(self) -> None:
+ config = BigNumberChartConfig(
+ chart_type="big_number",
+ metric=ColumnRef(name="revenue", aggregate="SUM"),
+ temporal_column="order_date",
+ show_trendline=True,
+ aggregation="sum",
+ )
+ form_data = map_big_number_config(config)
+ assert form_data["aggregation"] == "sum"
+
+ def test_with_aggregation_last_value(self) -> None:
+ config = BigNumberChartConfig(
+ chart_type="big_number",
+ metric=ColumnRef(name="revenue", aggregate="SUM"),
+ temporal_column="order_date",
+ show_trendline=True,
+ aggregation="LAST_VALUE",
+ )
+ form_data = map_big_number_config(config)
+ assert form_data["aggregation"] == "LAST_VALUE"
+
+ def test_aggregation_absent_when_not_set(self) -> None:
+ config = BigNumberChartConfig(
+ chart_type="big_number",
+ metric=ColumnRef(name="revenue", aggregate="SUM"),
+ temporal_column="order_date",
+ show_trendline=True,
+ )
+ form_data = map_big_number_config(config)
+ assert "aggregation" not in form_data
+
+ def test_aggregation_not_allowed_for_big_number_total(self) -> None:
Review Comment:
**Suggestion:** Add an inline docstring to this newly added test method
explaining the validation scenario it covers. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly introduced test method and it lacks a docstring. Because the
surrounding code is being changed and the rule requires new Python functions to
be documented inline, this is a real violation.
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e9fe11479bc7436e89b5c767c6b0040d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e9fe11479bc7436e89b5c767c6b0040d&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:** tests/unit_tests/mcp_service/chart/test_big_number_chart.py
**Line:** 379:379
**Comment:**
*Custom Rule: Add an inline docstring to this newly added test method
explaining the validation scenario it covers.
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%2F40775&comment_hash=0b4565e2629cfc05207baba732417e58b2db60e115fe72e8f3f269dfabaeefcb&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40775&comment_hash=0b4565e2629cfc05207baba732417e58b2db60e115fe72e8f3f269dfabaeefcb&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]
