bito-code-review[bot] commented on code in PR #43771:
URL: https://github.com/apache/superset/pull/43771#discussion_r3915661024
##########
superset/mcp_service/utils/response_utils.py:
##########
@@ -167,53 +171,208 @@ def build(self) -> Dict[str, str]:
STATS_ROW_CAP: int = 5000
+STATS_SAMPLE_VALUE_COUNT: int = 3
+STATS_TOTAL_WORK_CAP: int = 100_000
+
+_GENERIC_DATA_TYPE_NAMES: dict[int, str] = {
+ GenericDataType.NUMERIC: "numeric",
+ GenericDataType.STRING: "string",
+ GenericDataType.TEMPORAL: "temporal",
+ GenericDataType.BOOLEAN: "boolean",
+}
+_MAX_PROFILE_INTEGER_BITS = 4_096
+_MAX_PROFILE_STRING_LENGTH = 65_536
+
+
+@dataclass
+class _ColumnStatsBudget:
+ """One nested-node budget shared by all result columns."""
+
+ nodes: int = 0
+
+
+def data_column_stats_row_limit(row_count: int, column_count: int) -> int:
+ """Return a row sample whose aggregate top-level cell work is bounded."""
+ if row_count <= 0 or column_count <= 0:
+ return 0
+ return min(row_count, STATS_ROW_CAP, STATS_TOTAL_WORK_CAP // column_count)
+
+
+def _profile_value_identity( # noqa: C901
+ value: Any, budget: _ColumnStatsBudget
+) -> tuple[Any, ...] | None:
+ """Build a hook-free identity under the shared iterative node budget."""
+ tokens: list[Any] = []
+ stack: list[tuple[str, Any]] = [("value", value)]
+ active_containers: set[int] = set()
+ while stack:
+ action, item = stack.pop()
+ budget.nodes += 1
+ if budget.nodes > STATS_TOTAL_WORK_CAP:
+ return None
+ if action == "token":
+ tokens.append(item)
+ continue
+ if action == "leave":
+ active_containers.remove(id(item))
+ continue
+ if type(item) is list:
+ identity = id(item)
+ if identity in active_containers:
+ tokens.append(("cyclic_list",))
+ continue
+ active_containers.add(identity)
+ width = list.__len__(item)
+ tokens.append(("list", width))
+ stack.append(("leave", item))
+ stack.append(("token", "list_end"))
+ stack.extend(
+ ("value", list.__getitem__(item, index))
+ for index in range(width - 1, -1, -1)
+ )
+ continue
+ if type(item) is dict:
+ identity = id(item)
+ if identity in active_containers:
+ tokens.append(("cyclic_dict",))
+ continue
+ active_containers.add(identity)
+ entries = list(dict.items(item))
+ tokens.append(("dict", list.__len__(entries)))
+ stack.append(("leave", item))
+ stack.append(("token", "dict_end"))
+ for key, child in reversed(entries):
+ key_token = (
+ ("key", key)
+ if type(key) is str
+ and str.__len__(key) <= _MAX_PROFILE_STRING_LENGTH
+ else ("opaque_key", id(type(key)), id(key))
+ )
+ stack.append(("value", child))
+ stack.append(("token", key_token))
+ continue
+
+ value_type = type(item)
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>bool/int unique_count collision</b></div>
<div id="fix">
`_profile_value_identity` tokenizes bool as `("number", int(item), 1)`,
which collides with the int token `("number", item, 1)`. For a column mixing
booleans and integers (e.g. `[True, 1, False, 0]`), `unique_count` drops from 4
(old `str(val)` behavior) to 2. Use a distinct `"boolean"` tag to preserve type
distinction.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
elif value_type is bool:
tokens.append(("boolean", item))
elif value_type is int:
````
</div>
</details>
</div>
<small><i>Code Review Run #4d446c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/mcp_service/chart/tool/get_chart_data.py:
##########
@@ -167,6 +175,110 @@ def _rejected_requested_filter_columns(
}
_MAX_RECOMMENDATIONS = 4
+_MAX_COLUMN_PROFILE_CELLS = 100_000
+
+
+@dataclass
+class _ProfileBudget:
+ """One iterative node budget shared by every profiled column."""
+
+ nodes: int = 0
+
+
+def _canonical_profile_key(
+ value: Any, budget: _ProfileBudget
+) -> tuple[Any, ...] | None:
+ """Build a bounded identity from canonical JSON primitives, iteratively."""
+ tokens: list[Any] = []
+ stack: list[tuple[str, Any]] = [("value", value)]
+ while stack:
+ action, item = stack.pop()
+ budget.nodes += 1
+ if budget.nodes > _MAX_COLUMN_PROFILE_CELLS:
+ return None
+ if action == "end":
+ tokens.append(item)
+ continue
+ if type(item) is list:
+ tokens.append(("list", len(item)))
+ stack.append(("end", "list_end"))
+ stack.extend(
+ ("value", list.__getitem__(item, index))
+ for index in range(list.__len__(item) - 1, -1, -1)
+ )
+ continue
+ if type(item) is dict:
+ entries = sorted(dict.items(item))
+ tokens.append(("dict", len(entries)))
+ stack.append(("end", "dict_end"))
+ for key, child in reversed(entries):
+ stack.append(("value", child))
+ stack.append(("end", ("key", key)))
+ continue
+ value_type = type(item)
+ if value_type not in {type(None), bool, int, float, str}: # noqa: E721
+ raise TypeError("query result value was not canonicalized")
+ tokens.append((value_type.__name__, item))
+ return tuple(tokens)
+
+
+def _build_data_columns( # noqa: C901
+ data: list[dict[str, Any]],
+ raw_columns: list[str],
+ coltypes: list[Any] | None = None,
+) -> list[DataColumn]:
+ """Profile canonical rows under one nested-node budget."""
+ column_count = max(len(raw_columns), 1)
+ profile_row_limit = max(3, _MAX_COLUMN_PROFILE_CELLS // column_count)
+ budget = _ProfileBudget()
+ profiled_values: dict[str, list[Any]] = {column: [] for column in
raw_columns}
+ profiled_keys: dict[str, set[tuple[Any, ...]]] = {
+ column: set() for column in raw_columns
+ }
+ sampled_rows = 0
+ for row_offset in range(min(len(data), profile_row_limit)):
+ row = list.__getitem__(data, row_offset)
+ row_values: list[tuple[str, Any, tuple[Any, ...]]] = []
+ for col_name in raw_columns:
+ value = dict.get(row, col_name)
+ identity = _canonical_profile_key(value, budget)
+ if identity is None:
+ break
+ row_values.append((col_name, value, identity))
+ if len(row_values) != len(raw_columns):
+ break
+ for col_name, value, identity in row_values:
+ profiled_values[col_name].append(value)
+ profiled_keys[col_name].add(identity)
+ sampled_rows += 1
+
+ columns: list[DataColumn] = []
+ for idx, col_name in enumerate(raw_columns):
+ values = profiled_values[col_name]
+ sample_values = [value for value in values[:3] if value is not None]
+ data_type = "string"
+ if coltypes:
+ data_type = _GENERIC_TYPE_MAP.get(coltypes[idx], "string")
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Decimal type detection removed</b></div>
<div id="fix">
Removed `Decimal` from numeric type detection at line 261. Original code
checked for `{int, float, Decimal}`; now only checks `{int, float}`. This may
cause `Decimal` values to be misclassified as "string" instead of "numeric".
</div>
</div>
<small><i>Code Review Run #4d446c</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
