codeant-ai-for-open-source[bot] commented on code in PR #43303:
URL: https://github.com/apache/superset/pull/43303#discussion_r3805074973
##########
superset/commands/chart/importers/v1/__init__.py:
##########
@@ -103,7 +112,16 @@ def _import(
"datasource_name": dataset.table_name,
}
config = update_chart_config_dataset(config, dataset_dict)
+ # Capture the pre-import context state before `import_chart`
+ # mutates `config` (it synthesizes into
config["query_context"]).
+ had_query_context = bool(config.get("query_context"))
chart = import_chart(config, overwrite=overwrite)
+ if had_query_context:
+ n_preserved += 1
+ elif chart.query_context:
+ n_queryable += 1
+ else:
+ n_non_derivable += 1
Review Comment:
**Suggestion:** The summary counts any returned chart with a context as
newly `queryable` when the input lacked one. However, `import_chart()` returns
an existing chart immediately when `overwrite` is false, so a re-import can
report an existing chart's pre-existing context as synthesized by this import.
This makes the operator-facing counts incorrect; distinguish the early-return
case or report based on whether this import actually changed the chart. [logic
error]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Import logs overstate newly queryable charts.
- โ ๏ธ Operators receive inaccurate migration coverage metrics.
```
</details>
[](https://github.com/CodeAnt-AI/skills/blob/main/skills/codeant-resolve-pr-comments/SKILL.md)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/chart/importers/v1/__init__.py
**Line:** 117:124
**Comment:**
*Logic Error: The summary counts any returned chart with a context as
newly `queryable` when the input lacked one. However, `import_chart()` returns
an existing chart immediately when `overwrite` is false, so a re-import can
report an existing chart's pre-existing context as synthesized by this import.
This makes the operator-facing counts incorrect; distinguish the early-return
case or report based on whether this import actually changed the chart.
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%2F43303&comment_hash=6034ba2dc9fad84d01f3ff8c18b7ab977cdaa2accc03047c67a4edc9e3a654fe&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43303&comment_hash=6034ba2dc9fad84d01f3ff8c18b7ab977cdaa2accc03047c67a4edc9e3a654fe&reaction=dislike'>๐</a>
##########
superset/commands/chart/importers/v1/utils.py:
##########
@@ -70,6 +77,64 @@ def import_chart(
filter_chart_annotations(config)
+ # Synthesize a query_context for imported charts that arrive without one,
so
+ # the first `GET /api/v1/chart/{pk}/data/` returns data instead of HTTP 400
+ # "Chart has no query context saved" (issue #33615, ADR-013). Guarded on an
+ # ABSENT context so an existing/remapped one is never overwritten (FR-006).
+ #
+ # Two-tier derivation:
+ # 1. AUTHORITATIVE โ run the chart's real frontend `buildQuery` in V8
+ # (QueryContextGenerator) for byte-faithful parity with the UI.
+ # 2. FALLBACK โ a pure-Python generic derivation
+ # (`build_query_context_config`) when the V8 bundle / py_mini_racer is
+ # unavailable or the viz type is not (yet) covered by the bundle.
+ # Either way the datasource is taken from the importer-resolved id/type
ONLY,
+ # never a value carried in params (ADR-014 authz/RLS). A per-chart
derivation
+ # error must never abort the bundle (RISK-T03 / FR-004).
+ if not config.get("query_context"):
+ try:
+ params = config.get("params") or {}
+ viz_type = config["viz_type"]
+ datasource_id = config.get("datasource_id")
+ datasource_type = config.get("datasource_type", "table")
+
+ query_context_config = None
+ if datasource_id:
+ # form_data for the JS builder: the datasource is the
+ # importer-resolved id/type only (overwrite any incoming
+ # params.datasource โ never trust it; ADR-014).
+ js_params = {
+ **params,
+ "datasource": f"{datasource_id}__{datasource_type}",
+ }
+ query_context_config = get_query_context_generator().generate(
+ viz_type, js_params
+ )
+ if query_context_config is None:
+ query_context_config = build_query_context_config(
+ params, viz_type, datasource_id, datasource_type
Review Comment:
**Suggestion:** The query context is synthesized before `migrate_chart()`
rewrites legacy visualization types and parameters. For charts such as
`dual_line`, migration changes the chart to `mixed_timeseries` and constructs
multiple query objects, but the newly generated context remains based on the
old form data and query shape. Persisting this mismatched context can make the
imported chart query the wrong metrics, columns, or number of series. Generate
or migrate the query context after applying the chart migration. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ Migrated dual-line charts can query incomplete series.
- โ ๏ธ Chart data can differ from migrated visualization settings.
```
</details>
[](https://github.com/CodeAnt-AI/skills/blob/main/skills/codeant-resolve-pr-comments/SKILL.md)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/chart/importers/v1/utils.py
**Line:** 110:115
**Comment:**
*Api Mismatch: The query context is synthesized before
`migrate_chart()` rewrites legacy visualization types and parameters. For
charts such as `dual_line`, migration changes the chart to `mixed_timeseries`
and constructs multiple query objects, but the newly generated context remains
based on the old form data and query shape. Persisting this mismatched context
can make the imported chart query the wrong metrics, columns, or number of
series. Generate or migrate the query context after applying the chart
migration.
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%2F43303&comment_hash=41714b0c73c05b4efc5684588b2a602dcae41a09c469d2a6a857bb0302611cdb&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43303&comment_hash=41714b0c73c05b4efc5684588b2a602dcae41a09c469d2a6a857bb0302611cdb&reaction=dislike'>๐</a>
##########
superset/cli/charts.py:
##########
@@ -0,0 +1,142 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""CLI commands for charts (Apache Superset #33615)."""
+
+from __future__ import annotations
+
+import logging
+
+import click
+from flask.cli import with_appcontext
+
+logger = logging.getLogger(__name__)
+
+
[email protected]()
+def charts() -> None:
+ """Chart-related maintenance commands."""
+
+
[email protected]("backfill-query-context")
+@with_appcontext
[email protected](
+ "--dry-run",
+ is_flag=True,
+ default=False,
+ help="Report what would change without writing.",
+)
[email protected](
+ "--viz-type",
+ "viz_types",
+ multiple=True,
+ help="Restrict to these viz types (repeatable). Default: all.",
+)
[email protected](
+ "--batch-size",
+ type=int,
+ default=200,
+ show_default=True,
Review Comment:
**Suggestion:** `--batch-size` accepts zero and negative integers, then
passes the value to `yield_per()` and uses it as the commit threshold. A
non-positive value can make SQLAlchemy reject the query or cause the threshold
to be immediately true, resulting in invalid or per-row commit behavior.
Constrain the option to a positive integer before starting the backfill.
[possible bug]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ Backfill command can fail before processing charts.
- โ ๏ธ Invalid values can force per-chart commits.
```
</details>
[](https://github.com/CodeAnt-AI/skills/blob/main/skills/codeant-resolve-pr-comments/SKILL.md)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/cli/charts.py
**Line:** 49:52
**Comment:**
*Possible Bug: `--batch-size` accepts zero and negative integers, then
passes the value to `yield_per()` and uses it as the commit threshold. A
non-positive value can make SQLAlchemy reject the query or cause the threshold
to be immediately true, resulting in invalid or per-row commit behavior.
Constrain the option to a positive integer before starting the backfill.
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%2F43303&comment_hash=769ccbcdc1ab60ca6cfc0551c9c9059da0d9958cc58690549a7ac7e55f720e5d&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43303&comment_hash=769ccbcdc1ab60ca6cfc0551c9c9059da0d9958cc58690549a7ac7e55f720e5d&reaction=dislike'>๐</a>
##########
superset/commands/chart/query_context_builder.py:
##########
@@ -0,0 +1,186 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Derive a ``query_context`` payload from a chart's stored ``params``.
+
+Charts imported via a v1 ZIP bundle persist with ``Slice.query_context = NULL``
+(the importer never synthesizes one), so the first
+``GET /api/v1/chart/{pk}/data/`` returns HTTP 400 "Chart has no query context
+saved" (Apache Superset #33615). This module builds a valid ``query_context``
+payload from the chart's viz ``params`` + its importer-resolved datasource, so
+the imported row becomes queryable on first read โ or classifies the chart
+non-derivable and returns ``None`` (honest-fail; never a fabricated context).
+
+The function is **pure** (no DB / network / analytical-DB access). Its output,
+when passed to ``QueryContextFactory.create(**payload)``, constructs a valid
+``QueryContext``. See ADR-013 (synthesize-at-import) and ADR-014
+(datasource authz/RLS preservation).
+"""
+
+from __future__ import annotations
+
+from typing import Any
+
+# Datasource-less viz types render static/markdown content rather than a
+# datasource-backed query; a NULL query_context is the *correct* state for them
+# (FR-003), not a bug. Kept as a small, explicit, conservatively-expanded set.
+_NON_DATASOURCE_VIZ: frozenset[str] = frozenset({"markup", "handlebars",
"divider"})
+
+# Default row limit mirrors the query-object default used across the read path.
+_DEFAULT_ROW_LIMIT = 5000
+
+
+def _translate_adhoc_filters(
+ adhoc_filters: list[Any] | None,
+) -> tuple[list[dict[str, Any]], list[str], list[str]]:
+ """
+ Translate viz ``adhoc_filters`` into simple ``{col, op, val}`` filters.
+
+ - ``expressionType == "SIMPLE"`` โ a ``{col, op, val}`` filter.
+ - ``expressionType == "SQL"`` โ routed to ``extras.where`` (or
+ ``extras.having`` for a HAVING clause) so a hand-written SQL predicate is
+ preserved rather than lost.
+ - Anything that cannot be mapped cleanly (missing subject/operator,
malformed
+ entry) is dropped **without raising** (RISK-T05) โ an imported chart must
+ never abort its bundle over a single unmappable filter.
+
+ Returns ``(filters, where_expressions, having_expressions)``.
+ """
+ filters: list[dict[str, Any]] = []
+ where_expressions: list[str] = []
+ having_expressions: list[str] = []
+ for adhoc_filter in adhoc_filters or []:
+ if not isinstance(adhoc_filter, dict):
+ continue
+ expression_type = adhoc_filter.get("expressionType") or "SIMPLE"
+ if expression_type == "SIMPLE":
+ subject = adhoc_filter.get("subject")
+ operator = adhoc_filter.get("operator")
+ if subject and operator:
+ filters.append(
+ {
+ "col": subject,
+ "op": operator,
+ "val": adhoc_filter.get("comparator"),
+ }
+ )
+ # else: unmappable SIMPLE filter โ dropped, no crash (RISK-T05).
+ elif expression_type == "SQL":
+ sql_expression = adhoc_filter.get("sqlExpression")
+ if sql_expression:
+ clause = (adhoc_filter.get("clause") or "WHERE").upper()
+ if clause == "HAVING":
+ having_expressions.append(sql_expression)
+ else:
+ where_expressions.append(sql_expression)
Review Comment:
**Suggestion:** Malformed or unsupported adhoc filters are silently omitted
while a query context is still generated. This means a saved chart with an
unmappable filter can be imported with a broader query that returns rows the
chart's form data intended to exclude. Treat an unmappable filter as
non-derivable, or preserve it in a form the query processor understands, rather
than silently dropping it. [logic error]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Imported charts can display unintended rows.
- โ ๏ธ Saved query context no longer matches chart filters.
```
</details>
[](https://github.com/CodeAnt-AI/skills/blob/main/skills/codeant-resolve-pr-comments/SKILL.md)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/chart/query_context_builder.py
**Line:** 81:89
**Comment:**
*Logic Error: Malformed or unsupported adhoc filters are silently
omitted while a query context is still generated. This means a saved chart with
an unmappable filter can be imported with a broader query that returns rows the
chart's form data intended to exclude. Treat an unmappable filter as
non-derivable, or preserve it in a form the query processor understands, rather
than silently dropping it.
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%2F43303&comment_hash=a1b1beac5bcf8169c35ad3cb6397aae66fad7624f2998ea937b566020b03156f&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43303&comment_hash=a1b1beac5bcf8169c35ad3cb6397aae66fad7624f2998ea937b566020b03156f&reaction=dislike'>๐</a>
##########
superset/commands/chart/query_context_builder.py:
##########
@@ -0,0 +1,186 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Derive a ``query_context`` payload from a chart's stored ``params``.
+
+Charts imported via a v1 ZIP bundle persist with ``Slice.query_context = NULL``
+(the importer never synthesizes one), so the first
+``GET /api/v1/chart/{pk}/data/`` returns HTTP 400 "Chart has no query context
+saved" (Apache Superset #33615). This module builds a valid ``query_context``
+payload from the chart's viz ``params`` + its importer-resolved datasource, so
+the imported row becomes queryable on first read โ or classifies the chart
+non-derivable and returns ``None`` (honest-fail; never a fabricated context).
+
+The function is **pure** (no DB / network / analytical-DB access). Its output,
+when passed to ``QueryContextFactory.create(**payload)``, constructs a valid
+``QueryContext``. See ADR-013 (synthesize-at-import) and ADR-014
+(datasource authz/RLS preservation).
+"""
+
+from __future__ import annotations
+
+from typing import Any
+
+# Datasource-less viz types render static/markdown content rather than a
+# datasource-backed query; a NULL query_context is the *correct* state for them
+# (FR-003), not a bug. Kept as a small, explicit, conservatively-expanded set.
+_NON_DATASOURCE_VIZ: frozenset[str] = frozenset({"markup", "handlebars",
"divider"})
+
+# Default row limit mirrors the query-object default used across the read path.
+_DEFAULT_ROW_LIMIT = 5000
+
+
+def _translate_adhoc_filters(
+ adhoc_filters: list[Any] | None,
+) -> tuple[list[dict[str, Any]], list[str], list[str]]:
+ """
+ Translate viz ``adhoc_filters`` into simple ``{col, op, val}`` filters.
+
+ - ``expressionType == "SIMPLE"`` โ a ``{col, op, val}`` filter.
+ - ``expressionType == "SQL"`` โ routed to ``extras.where`` (or
+ ``extras.having`` for a HAVING clause) so a hand-written SQL predicate is
+ preserved rather than lost.
+ - Anything that cannot be mapped cleanly (missing subject/operator,
malformed
+ entry) is dropped **without raising** (RISK-T05) โ an imported chart must
+ never abort its bundle over a single unmappable filter.
+
+ Returns ``(filters, where_expressions, having_expressions)``.
+ """
+ filters: list[dict[str, Any]] = []
+ where_expressions: list[str] = []
+ having_expressions: list[str] = []
+ for adhoc_filter in adhoc_filters or []:
+ if not isinstance(adhoc_filter, dict):
+ continue
+ expression_type = adhoc_filter.get("expressionType") or "SIMPLE"
+ if expression_type == "SIMPLE":
+ subject = adhoc_filter.get("subject")
+ operator = adhoc_filter.get("operator")
+ if subject and operator:
+ filters.append(
+ {
+ "col": subject,
+ "op": operator,
+ "val": adhoc_filter.get("comparator"),
+ }
+ )
+ # else: unmappable SIMPLE filter โ dropped, no crash (RISK-T05).
+ elif expression_type == "SQL":
+ sql_expression = adhoc_filter.get("sqlExpression")
+ if sql_expression:
+ clause = (adhoc_filter.get("clause") or "WHERE").upper()
+ if clause == "HAVING":
+ having_expressions.append(sql_expression)
+ else:
+ where_expressions.append(sql_expression)
+ return filters, where_expressions, having_expressions
+
+
+def _derive_orderby(params: dict[str, Any]) -> list[list[Any]]:
+ """
+ Best-effort ordering from ``params`` (FR-002).
+
+ Handles an explicit ``orderby`` list (either ``[[col, asc_bool], ...]`` or
a
+ flat list of expressions) and a single sort metric
+ (``timeseries_limit_metric`` / ``sort_by_metric``). Falls back to no
+ ordering when nothing is derivable โ the query builder supplies defaults.
+ """
+ order_asc = not params.get("order_desc", True)
+
+ orderby = params.get("orderby")
+ if isinstance(orderby, list) and orderby:
+ normalized: list[list[Any]] = []
+ for entry in orderby:
+ if isinstance(entry, (list, tuple)) and len(entry) == 2:
+ normalized.append([entry[0], bool(entry[1])])
+ else:
+ normalized.append([entry, order_asc])
+ return normalized
+
+ sort_metric = params.get("timeseries_limit_metric") or params.get(
+ "sort_by_metric"
+ )
+ if sort_metric:
+ return [[sort_metric, order_asc]]
+ return []
+
+
+def build_query_context_config(
+ params: dict[str, Any] | None,
+ viz_type: str,
+ datasource_id: int | None,
+ datasource_type: str = "table",
+) -> dict[str, Any] | None:
+ """
+ Map a chart's ``params`` + resolved datasource to a ``query_context``
payload.
+
+ :param params: the chart's viz form-data (a dict at ``import_chart`` time).
+ :param viz_type: the chart's viz type (classification input).
+ :param datasource_id: the importer-resolved datasource id. **Datasource is
+ taken from this argument only, never from ``params`` (ADR-014 /
RISK-T02)**
+ so the synthesized context names the same real datasource the authz
layer
+ vets at read time.
+ :param datasource_type: the datasource type โ ``"table"`` on import.
+ :returns: a ``query_context`` payload dict whose keys are the kwargs of
+ ``QueryContextFactory.create``, or ``None`` when the chart is
+ non-derivable (FR-003): no datasource, nothing to query, or a
+ datasource-less viz type. Never returns a fabricated/invalid context.
+ """
+ params = params or {}
+
+ # FR-003 non-derivable classification โ return None, importer leaves NULL.
+ if not datasource_id or viz_type in _NON_DATASOURCE_VIZ:
+ return None
+ metrics = params.get("metrics") or []
+ # `groupby` is the deprecated alias of `columns`.
+ columns = params.get("columns") or params.get("groupby") or []
+ if not metrics and not columns:
+ return None
+
+ filters, where_expressions, having_expressions = _translate_adhoc_filters(
+ params.get("adhoc_filters", [])
+ )
+
+ query_object = {
+ "time_range": params.get("time_range", " : "),
+ "granularity": params.get("granularity_sqla") or
params.get("granularity"),
+ "filters": filters,
+ "extras": {
+ "time_grain_sqla": params.get("time_grain_sqla"),
+ "having": " AND ".join(having_expressions),
+ "where": " AND ".join(where_expressions),
+ },
+ "applied_time_extras": {},
+ "columns": columns,
+ "metrics": metrics,
+ "orderby": _derive_orderby(params),
+ "annotation_layers": [],
+ "row_limit": params.get("row_limit", _DEFAULT_ROW_LIMIT),
Review Comment:
**Suggestion:** The fallback hard-codes an absent `row_limit` to 5000, but
`QueryObjectFactory` uses the configured `ROW_LIMIT` for normal full-result
queries, which is 50000 in the default configuration. Imported charts without
an explicit row limit will therefore silently return fewer rows than the same
chart queried through the normal path. Omit the field or derive the default
from the configured query limit instead of using a separate constant. [logic
error]
<details>
<summary><b>Severity Level:</b> Major โ ๏ธ</summary>
```mdx
- โ ๏ธ Imported charts silently return fewer result rows.
- โ ๏ธ Large table and visualization results are truncated.
```
</details>
[](https://github.com/CodeAnt-AI/skills/blob/main/skills/codeant-resolve-pr-comments/SKILL.md)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/chart/query_context_builder.py
**Line:** 172:172
**Comment:**
*Logic Error: The fallback hard-codes an absent `row_limit` to 5000,
but `QueryObjectFactory` uses the configured `ROW_LIMIT` for normal full-result
queries, which is 50000 in the default configuration. Imported charts without
an explicit row limit will therefore silently return fewer rows than the same
chart queried through the normal path. Omit the field or derive the default
from the configured query limit instead of using a separate constant.
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%2F43303&comment_hash=290e6620427f8eb7c66a53abb188e30514d121bdef60361bb4be6cb170cdcc2e&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43303&comment_hash=290e6620427f8eb7c66a53abb188e30514d121bdef60361bb4be6cb170cdcc2e&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]
