Commit ·
b40fa76
1
Parent(s): 776fae3
fix(analyze_aggregate): reject non-string group_by (CASE/binning) keys
Browse filesWhen asked to bucket a numeric column ("group uptime into high/medium/low"), the
planner emitted a CASE expression as a group_by entry (a dict). df.groupby then
crashed with a bare "TypeError: unhashable type: 'dict'", surfaced to the user as
a misleading "technical issue". Guard both layers, same as the data-handoff fix:
the validator (new Check 8d) rejects any non-string group_by key so the retry
loop re-prompts the planner, and analyze_aggregate raises a clear
UnsupportedGroupByError instead of the opaque TypeError if one still reaches it.
Derived bucketing belongs to analyze_segment, not analyze_aggregate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
src/agents/planner/validator.py
CHANGED
|
@@ -142,6 +142,23 @@ class PlannerValidator:
|
|
| 142 |
"use analyze_descriptive instead."
|
| 143 |
)
|
| 144 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 145 |
# Check 3 — concrete source_id args must exist in the catalog.
|
| 146 |
src = call.args.get("source_id")
|
| 147 |
if isinstance(src, str) and not _is_placeholder(src):
|
|
|
|
| 142 |
"use analyze_descriptive instead."
|
| 143 |
)
|
| 144 |
|
| 145 |
+
# Check 8d — group_by keys must be plain column-name strings. A
|
| 146 |
+
# derived grouping (a CASE/binning expression emitted as a dict)
|
| 147 |
+
# is unhashable and crashes df.groupby at execution ("unhashable
|
| 148 |
+
# type: 'dict'"). Reject it here so the planner is re-prompted;
|
| 149 |
+
# bucketing a numeric column into ranges belongs to
|
| 150 |
+
# analyze_segment, not smuggled through analyze_aggregate.
|
| 151 |
+
group_by = call.args.get("group_by")
|
| 152 |
+
if isinstance(group_by, list):
|
| 153 |
+
bad_keys = [g for g in group_by if not isinstance(g, str)]
|
| 154 |
+
if bad_keys:
|
| 155 |
+
raise PlannerValidationError(
|
| 156 |
+
f"task {task.id}: analyze_aggregate group_by must be "
|
| 157 |
+
f"column names (strings); got non-string entr(ies) "
|
| 158 |
+
f"{bad_keys}. Derived groupings (CASE/binning) are not "
|
| 159 |
+
"supported — group by an existing column."
|
| 160 |
+
)
|
| 161 |
+
|
| 162 |
# Check 3 — concrete source_id args must exist in the catalog.
|
| 163 |
src = call.args.get("source_id")
|
| 164 |
if isinstance(src, str) and not _is_placeholder(src):
|
src/tools/analytics/aggregation.py
CHANGED
|
@@ -27,6 +27,17 @@ class UnsupportedAggregationError(ValueError):
|
|
| 27 |
"""Requested aggregation is not in SUPPORTED_AGGS (maps to UNSUPPORTED_AGG)."""
|
| 28 |
|
| 29 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 30 |
def _clean(value: object) -> object:
|
| 31 |
"""Convert numpy/pandas scalars to plain Python so the output is JSON-clean.
|
| 32 |
|
|
@@ -103,6 +114,17 @@ def analyze_aggregate(
|
|
| 103 |
"""
|
| 104 |
group_by = group_by or []
|
| 105 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 106 |
# Validate columns first (fail-fast on caller mistakes).
|
| 107 |
referenced = list(group_by) + list(aggregations.keys())
|
| 108 |
missing = [c for c in referenced if c not in df.columns]
|
|
|
|
| 27 |
"""Requested aggregation is not in SUPPORTED_AGGS (maps to UNSUPPORTED_AGG)."""
|
| 28 |
|
| 29 |
|
| 30 |
+
class UnsupportedGroupByError(ValueError):
|
| 31 |
+
"""A group_by entry is not a plain column name (maps to UNSUPPORTED_GROUP_BY).
|
| 32 |
+
|
| 33 |
+
group_by keys must be column-name strings. A derived grouping — e.g. a CASE /
|
| 34 |
+
binning expression the planner emits as a dict — is unhashable and would crash
|
| 35 |
+
`df.groupby` with a bare "unhashable type: 'dict'"; raising here turns it into a
|
| 36 |
+
clear, actionable error instead. Bucketing a numeric column into ranges belongs
|
| 37 |
+
to analyze_segment, not analyze_aggregate.
|
| 38 |
+
"""
|
| 39 |
+
|
| 40 |
+
|
| 41 |
def _clean(value: object) -> object:
|
| 42 |
"""Convert numpy/pandas scalars to plain Python so the output is JSON-clean.
|
| 43 |
|
|
|
|
| 114 |
"""
|
| 115 |
group_by = group_by or []
|
| 116 |
|
| 117 |
+
# group_by keys must be plain column names. A non-string entry (e.g. a CASE /
|
| 118 |
+
# binning dict) is unhashable and would otherwise crash the membership check or
|
| 119 |
+
# df.groupby with an opaque "unhashable type: 'dict'".
|
| 120 |
+
non_str = [g for g in group_by if not isinstance(g, str)]
|
| 121 |
+
if non_str:
|
| 122 |
+
raise UnsupportedGroupByError(
|
| 123 |
+
f"group_by entries must be column names (strings); got non-string "
|
| 124 |
+
f"entr(ies) {non_str}. Derived groupings (e.g. CASE/binning expressions) "
|
| 125 |
+
"are not supported by analyze_aggregate — group by an existing column."
|
| 126 |
+
)
|
| 127 |
+
|
| 128 |
# Validate columns first (fail-fast on caller mistakes).
|
| 129 |
referenced = list(group_by) + list(aggregations.keys())
|
| 130 |
missing = [c for c in referenced if c not in df.columns]
|