drill_down: filter invalid dimensions before they reach the composer

api/analyses/drill_down/types.py

@@ -2,9 +2,12 @@
 
 from __future__ import annotations
 
+import logging
 from dataclasses import dataclass, field
 from typing import Any
 
+logger = logging.getLogger("nvi.analyses.drill_down.types")
+
 DEFAULT_DIMENSIONS: list[str] = ["A2", "A3", "A11", "A12", "A13", "A14"]
 DEFAULT_METRIC: str = "loan_default_rate"
 DEFAULT_MAX_ITERS: int = 3
@@ -43,14 +46,49 @@ class DrillDownArgs:
 
     @classmethod
     def from_raw(cls, raw: dict[str, Any], default_question: str) -> "DrillDownArgs":
+        raw_dims = raw.get("dimensions") or list(DEFAULT_DIMENSIONS)
         return cls(
             question=raw.get("question") or default_question,
             metric=raw.get("metric") or DEFAULT_METRIC,
-            dimensions=raw.get("dimensions") or list(DEFAULT_DIMENSIONS),
+            dimensions=_filter_to_valid_columns(raw_dims),
             max_iters=int(raw.get("max_iters") or DEFAULT_MAX_ITERS),
         )
 
 
+def _filter_to_valid_columns(dims: list[str]) -> list[str]:
+    """Drop any candidate dimension that isn't a real column ref. The
+    planner sometimes conflates metric names (e.g. 'unemployment_rate_96')
+    with column names — they look the same shape but only one can be
+    grouped by. We drop the bad ones here so `decide_next` never picks
+    them and the composer never sees them. Falls back to DEFAULT_DIMENSIONS
+    if everything got filtered out (better than running with no candidates)."""
+    # Import lazily — recon access from a types module would create a cycle
+    # if recon is being rebuilt; deferring it sidesteps that.
+    from api.recon import load_recon
+
+    recon = load_recon()
+    kept: list[str] = []
+    dropped: list[tuple[str, str]] = []
+    for d in dims:
+        try:
+            recon.resolve_column(d)
+            kept.append(d)
+        except ValueError as e:
+            dropped.append((d, str(e)))
+    if dropped:
+        logger.warning(
+            "drill_down dimensions filtered (%d kept, %d dropped): %s",
+            len(kept), len(dropped),
+            ", ".join(f"{d!r} ({reason})" for d, reason in dropped),
+        )
+    if not kept:
+        logger.warning(
+            "all proposed dimensions invalid; falling back to DEFAULT_DIMENSIONS"
+        )
+        return list(DEFAULT_DIMENSIONS)
+    return kept
+
+
 @dataclass
 class Slice:
     """One iteration's slice: the chosen dimension, the SQL it produced,

api/prompts/planner.system.txt

@@ -25,3 +25,8 @@ Rules:
 - Most questions are single-step. Only chain analyses when the second genuinely needs the first's output.
 - Comparing two periods is ONE `compare_periods` step, not two `direct_answer` steps.
 - A step's `fallback` is optional. Include one when the primary analysis is speculative (e.g. an exploratory `drill_down` that might find nothing useful) and there's a safer Analysis that can still produce an answer. The fallback runs only if the primary errors or yields no finding. Fallbacks must not themselves have fallbacks.
+
+Distinguishing metrics from dimensions:
+- A `metric` is a name from the metric catalog above — what gets aggregated. Example: `loan_default_rate`.
+- A `dimension` (in drill_down args) is a raw COLUMN name from a warehouse table — what you group by. Example: `A2` (district name), `status` (loan status), `frequency` (statement frequency). NEVER put a metric name in the dimensions list.
+- If unsure whether a name is a metric or a column, check the metric catalog: anything listed there is a metric, not a dimension.