codeant-ai-for-open-source[bot] commented on code in PR #44679:
URL: https://github.com/apache/superset/pull/44679#discussion_r4278634448
##########
superset/charts/client_processing.py:
##########
@@ -479,6 +479,133 @@ def _apply_show_values_as( # pylint:
disable=too-many-arguments
return numeric / denominator.replace(0, np.nan)
+def _reduce_result_values(values: pd.Series, aggregate: str) -> Any:
+ """Reduce original metric results without aggregating intermediate
summaries."""
+ if values.empty:
+ return None
+ name = aggregate.split(" as Fraction of ")[0]
+ if name == "Count":
+ return len(values)
+ if name == "Count Unique Values":
+ return values.nunique(dropna=False)
+ if name == "List Unique Values":
+ return ", ".join(sorted({str(value) for value in values}))
+ if name in {"First", "Last"}:
+ present = values.dropna()
+ return present.iloc[0 if name == "First" else -1] if len(present) else
None
Review Comment:
**Suggestion:** For a group whose last result is NULL, this returns an
earlier non-null value, while the browser returns NULL, so the tabular export
disagrees.
**Assessment:** π `Major` Β· π `Occurrence: Sometimes` Β· π·οΈ `Api mismatch`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=088267417201437ebc71be754aea1779&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=088267417201437ebc71be754aea1779&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/client_processing.py
**Line:** 494:495
**Comment:**
*Api Mismatch: For a group whose last result is NULL, this returns an
earlier non-null value, while the browser returns NULL, so the tabular export
disagrees.
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%2F44679&comment_hash=e3f1ec3c08f9c1feda4d812fe524b2720eb9d460a6e56c281eea69e18f8476c9&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44679&comment_hash=e3f1ec3c08f9c1feda4d812fe524b2720eb9d460a6e56c281eea69e18f8476c9&reaction=dislike'>π</a>
##########
superset/charts/client_processing.py:
##########
@@ -479,6 +479,133 @@ def _apply_show_values_as( # pylint:
disable=too-many-arguments
return numeric / denominator.replace(0, np.nan)
+def _reduce_result_values(values: pd.Series, aggregate: str) -> Any:
+ """Reduce original metric results without aggregating intermediate
summaries."""
+ if values.empty:
+ return None
+ name = aggregate.split(" as Fraction of ")[0]
+ if name == "Count":
+ return len(values)
+ if name == "Count Unique Values":
+ return values.nunique(dropna=False)
+ if name == "List Unique Values":
+ return ", ".join(sorted({str(value) for value in values}))
+ if name in {"First", "Last"}:
+ present = values.dropna()
+ return present.iloc[0 if name == "First" else -1] if len(present) else
None
+ numeric = pd.to_numeric(values, errors="coerce").dropna()
Review Comment:
**Suggestion:** This drops NULL observations, but the browser converts them
to zero for numeric reducers, so nullable exports show different averages,
medians, extrema, and variances.
**Assessment:** π `Major` Β· π `Occurrence: Sometimes` Β· π·οΈ `Api mismatch`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c0a4487da36f4d5eb709f88ed448ad2c&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=c0a4487da36f4d5eb709f88ed448ad2c&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/client_processing.py
**Line:** 496:496
**Comment:**
*Api Mismatch: This drops NULL observations, but the browser converts
them to zero for numeric reducers, so nullable exports show different averages,
medians, extrema, and variances.
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%2F44679&comment_hash=fac206bf839cb7d94b69148110c083d4ac9ff91ecd89462da1cf8091c5278acc&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44679&comment_hash=fac206bf839cb7d94b69148110c083d4ac9ff91ecd89462da1cf8091c5278acc&reaction=dislike'>π</a>
##########
superset/charts/client_processing.py:
##########
@@ -978,6 +1162,7 @@ def build_pivot_currency_context(
currency_pivot_options = {
**pivot_options,
"aggfunc": CURRENCY_CONTEXT_AGGREGATION,
+ "result_aggregation": False,
Review Comment:
**Suggestion:** With subtotals enabled but grand totals disabled, this
prevents the currency-context pivot from creating subtotal labels, so AUTO
formatting loses each subtotalβs currency context.
**Assessment:** π `Major` Β· π `Occurrence: Rarely` Β· π·οΈ `Incomplete
implementation`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=33e1894fde3e4a43ae5985b4356e9cbb&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=33e1894fde3e4a43ae5985b4356e9cbb&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/client_processing.py
**Line:** 1165:1165
**Comment:**
*Incomplete Implementation: With subtotals enabled but grand totals
disabled, this prevents the currency-context pivot from creating subtotal
labels, so AUTO formatting loses each subtotalβs currency context.
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%2F44679&comment_hash=0dd639600a8da830501bb93739afdb54e44a97f10a689d84fc2bd2037dd8b8e6&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44679&comment_hash=0dd639600a8da830501bb93739afdb54e44a97f10a689d84fc2bd2037dd8b8e6&reaction=dislike'>π</a>
##########
superset/charts/client_processing.py:
##########
@@ -479,6 +479,133 @@ def _apply_show_values_as( # pylint:
disable=too-many-arguments
return numeric / denominator.replace(0, np.nan)
+def _reduce_result_values(values: pd.Series, aggregate: str) -> Any:
+ """Reduce original metric results without aggregating intermediate
summaries."""
+ if values.empty:
+ return None
+ name = aggregate.split(" as Fraction of ")[0]
+ if name == "Count":
+ return len(values)
+ if name == "Count Unique Values":
+ return values.nunique(dropna=False)
+ if name == "List Unique Values":
+ return ", ".join(sorted({str(value) for value in values}))
+ if name in {"First", "Last"}:
+ present = values.dropna()
+ return present.iloc[0 if name == "First" else -1] if len(present) else
None
+ numeric = pd.to_numeric(values, errors="coerce").dropna()
+ if name == "Sum":
+ text = values.dropna()
+ return text.iloc[-1] if numeric.empty and len(text) else numeric.sum()
+ if numeric.empty:
+ return None
+ if name == "Sample Variance":
+ return numeric.var(ddof=1) if len(numeric) > 1 else 0
+ if name == "Sample Standard Deviation":
+ return numeric.std(ddof=1) if len(numeric) > 1 else 0
+ return {
+ "Average": numeric.mean,
+ "Median": numeric.median,
+ "Minimum": numeric.min,
+ "Maximum": numeric.max,
+ }[name]()
+
+
+def _group_result_records(
+ source: pd.DataFrame, dimensions: list[str]
+) -> dict[tuple[str, ...], pd.DataFrame]:
+ """Index contributing records by the dimension prefix of a summary
scope."""
+ if not dimensions:
+ return {(): source}
+ keys = [source[dim].fillna("SUPERSET_PANDAS_NAN").astype(str) for dim in
dimensions]
+ return {
+ key if isinstance(key, tuple) else (key,): group
+ for key, group in source.groupby(keys, sort=False, dropna=False)
+ }
+
+
+def _apply_result_aggregation(
+ frame: pd.DataFrame,
+ source: pd.DataFrame,
+ rows: list[str],
+ columns: list[str],
+ metrics: list[str],
+ metric_level: int,
+ row_depths: dict[Any, int],
+ column_depths: dict[Any, int],
+ aggregate: str,
+ show_values_as: Optional[str],
+ metrics_on_rows: bool,
+) -> pd.DataFrame:
+ """Calculate every scope from its original contributing query results."""
+
+ if not set(metrics).issubset(source.columns):
+ return frame
+
+ grouped_sources: dict[tuple[int, int], dict[tuple[str, ...],
pd.DataFrame]] = {}
+
+ def contributing(
+ row: Any,
+ column: Any,
+ row_depth: int,
+ column_depth: int,
+ selected_metrics: list[str],
+ ) -> pd.Series:
+ level = (row_depth, column_depth)
+ if level not in grouped_sources:
+ grouped_sources[level] = _group_result_records(
+ source, rows[:row_depth] + columns[:column_depth]
+ )
+ key = _rollup_key(row, row_depth, metric_level, is_column=False) +
_rollup_key(
+ column,
+ column_depth,
+ metric_level,
+ is_column=True,
+ )
+ selected = grouped_sources[level].get(key, source.iloc[:0])
+ # Row-major order matches the browser's metric unpivoting.
+ return pd.Series(selected[selected_metrics].to_numpy().ravel(),
dtype=object)
+
+ old_fraction = aggregate.partition(" as Fraction of ")[2]
+ fraction = {
+ "Total": "percent_total",
+ "Rows": "percent_row",
+ "Columns": "percent_col",
+ }.get(old_fraction) or show_values_as
+ for ri, row in enumerate(frame.index):
+ for ci, column in enumerate(frame.columns):
+ rd = row_depths.get(row, len(rows))
+ cd = column_depths.get(column, len(columns))
+ metric = _metric_of_column(column, metric_level)
+ selected_metrics = (
+ [metric] if metric in metrics else list(dict.fromkeys(metrics))
+ )
+ # Match the browser's mixed-metric guard: an axis/corner that
+ # combines different metrics has no meaningful single summary.
+ if len(selected_metrics) != 1:
+ frame.iloc[ri, ci] = None
+ continue
+ value = _reduce_result_values(
+ contributing(row, column, rd, cd, selected_metrics), aggregate
+ )
+ if fraction in SHOW_VALUES_AS_PERCENT_MODES:
+ dr, dc = rd, cd
+ if fraction == "percent_total":
+ dr, dc = 0, 0
+ elif (fraction == "percent_row") != metrics_on_rows:
+ dc = 0
+ else:
+ dr = 0
+ denominator = _reduce_result_values(
+ contributing(row, column, dr, dc, selected_metrics),
aggregate
+ )
+ value = (
+ value / denominator if value is not None and denominator
else None
+ )
Review Comment:
**Suggestion:** When both options are set, this applies `showValuesAs` after
the result reducer, while the browser gives the result reducer precedence, so
chart and export values differ.
**Assessment:** π `Major` Β· π `Occurrence: Sometimes` Β· π·οΈ `Api mismatch`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=438db625ff824af3bc348ed58a28af1b&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=438db625ff824af3bc348ed58a28af1b&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/client_processing.py
**Line:** 591:604
**Comment:**
*Api Mismatch: When both options are set, this applies `showValuesAs`
after the result reducer, while the browser gives the result reducer
precedence, so chart and export values differ.
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%2F44679&comment_hash=3ca9aea82dd0d2c9ce8e4ecf08be1d07a7074d1a01e2946c1539125e39309842&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44679&comment_hash=3ca9aea82dd0d2c9ce8e4ecf08be1d07a7074d1a01e2946c1539125e39309842&reaction=dislike'>π</a>
##########
superset/charts/client_processing.py:
##########
@@ -479,6 +479,133 @@ def _apply_show_values_as( # pylint:
disable=too-many-arguments
return numeric / denominator.replace(0, np.nan)
+def _reduce_result_values(values: pd.Series, aggregate: str) -> Any:
+ """Reduce original metric results without aggregating intermediate
summaries."""
+ if values.empty:
+ return None
+ name = aggregate.split(" as Fraction of ")[0]
+ if name == "Count":
+ return len(values)
+ if name == "Count Unique Values":
+ return values.nunique(dropna=False)
+ if name == "List Unique Values":
+ return ", ".join(sorted({str(value) for value in values}))
+ if name in {"First", "Last"}:
+ present = values.dropna()
+ return present.iloc[0 if name == "First" else -1] if len(present) else
None
+ numeric = pd.to_numeric(values, errors="coerce").dropna()
+ if name == "Sum":
+ text = values.dropna()
+ return text.iloc[-1] if numeric.empty and len(text) else numeric.sum()
+ if numeric.empty:
+ return None
+ if name == "Sample Variance":
+ return numeric.var(ddof=1) if len(numeric) > 1 else 0
+ if name == "Sample Standard Deviation":
+ return numeric.std(ddof=1) if len(numeric) > 1 else 0
+ return {
+ "Average": numeric.mean,
+ "Median": numeric.median,
+ "Minimum": numeric.min,
+ "Maximum": numeric.max,
+ }[name]()
+
+
+def _group_result_records(
+ source: pd.DataFrame, dimensions: list[str]
+) -> dict[tuple[str, ...], pd.DataFrame]:
+ """Index contributing records by the dimension prefix of a summary
scope."""
+ if not dimensions:
+ return {(): source}
+ keys = [source[dim].fillna("SUPERSET_PANDAS_NAN").astype(str) for dim in
dimensions]
+ return {
+ key if isinstance(key, tuple) else (key,): group
+ for key, group in source.groupby(keys, sort=False, dropna=False)
+ }
Review Comment:
**Suggestion:** Converting dimension keys to strings merges distinct values
such as integer `1` and string `1`, so summaries for those groups include each
otherβs records.
**Assessment:** π `Major` Β· π `Occurrence: Rarely` Β· π·οΈ `Type error`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bd3330af87fa4da4be5c252872fda410&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=bd3330af87fa4da4be5c252872fda410&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/client_processing.py
**Line:** 520:524
**Comment:**
*Type Error: Converting dimension keys to strings merges distinct
values such as integer `1` and string `1`, so summaries for those groups
include each otherβs records.
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%2F44679&comment_hash=05e58cd050b0c148a42e65cd5c33d83e989f086df8bfc2b43ab6f2f6fe658005&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44679&comment_hash=05e58cd050b0c148a42e65cd5c33d83e989f086df8bfc2b43ab6f2f6fe658005&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]
