fix(clickhouse-chat-agent): address review feedback

- Keep data-derived keys out of the injected chart stylesheet: chart configs
  carry labels only (colors are applied directly on the marks), and ChartStyle
  sanitizes keys before interpolating CSS var names
- Wrap the json-render Renderer in an error boundary so a bad spec degrades
  to an inline message instead of crashing the chat
- Nested optional catalog fields (series labels, map point labels/values) use
  .nullish() so the model can omit them, as the prompt says it may
- Validate longitude range alongside latitude in PointMap
- Fix Tailwind v4 CSS-variable shorthand on the tooltip indicator

Author: matt-aitken

clickhouse-chat-agent/src/components/charts.tsx

@@ -33,10 +33,11 @@ function seriesColor(index: number): string {
   return `var(--chart-${(index % 5) + 1})`;
 }
 
+// Config carries labels only. Colors are applied directly on the marks
+// (Bar/Line/Cell fills) — keeping model/data-derived keys out of the
+// stylesheet that ChartStyle would otherwise inject them into.
 function buildConfig(series: Series[]): ChartConfig {
-  return Object.fromEntries(
-    series.map((s, i) => [s.dataKey, { label: s.label ?? s.dataKey, color: seriesColor(i) }])
-  );
+  return Object.fromEntries(series.map((s) => [s.dataKey, { label: s.label ?? s.dataKey }]));
 }
 
 function ChartFrame({ title, children }: { title?: string | null; children: React.ReactNode }) {
@@ -176,7 +177,7 @@ export function PieChartView({
   title?: string | null;
 }) {
   const config: ChartConfig = Object.fromEntries(
-    data.map((row, i) => [String(row[nameKey]), { label: String(row[nameKey]), color: seriesColor(i) }])
+    data.map((row) => [String(row[nameKey]), { label: String(row[nameKey]) }])
   );
 
   return (

clickhouse-chat-agent/src/components/point-map.tsx

@@ -16,7 +16,11 @@ const MAX_SIZE = 30;
 
 export function PointMapView({ points, title }: { points: Point[]; title?: string | null }) {
   const valid = points.filter(
-    (p) => Number.isFinite(p.lat) && Number.isFinite(p.lng) && Math.abs(p.lat) <= 90
+    (p) =>
+      Number.isFinite(p.lat) &&
+      Number.isFinite(p.lng) &&
+      Math.abs(p.lat) <= 90 &&
+      Math.abs(p.lng) <= 180
   );
   if (valid.length === 0) {
     return <div className="text-sm text-muted-foreground">No mappable points.</div>;

clickhouse-chat-agent/src/components/ui/chart.tsx

@@ -88,7 +88,10 @@ ${colorConfig
     const color =
       itemConfig.theme?.[theme as keyof typeof itemConfig.theme] ||
       itemConfig.color
-    return color ? `  --color-${key}: ${color};` : null
+    // Config keys can be data-derived — strip anything that could break out
+    // of the stylesheet before interpolating into the CSS var name.
+    const safeKey = key.replace(/[^a-zA-Z0-9_-]/g, "_")
+    return color ? `  --color-${safeKey}: ${color};` : null
   })
   .join("\n")}
 }
@@ -208,7 +211,7 @@ const ChartTooltipContent = React.forwardRef<
                       !hideIndicator && (
                         <div
                           className={cn(
-                            "shrink-0 rounded-[2px] border-[--color-border] bg-[--color-bg]",
+                            "shrink-0 rounded-[2px] border-(--color-border) bg-(--color-bg)",
                             {
                               "h-2.5 w-2.5": indicator === "dot",
                               "w-1": indicator === "line",

clickhouse-chat-agent/src/components/visualization.tsx

@@ -1,15 +1,40 @@
 "use client";
 
 import { JSONUIProvider, Renderer } from "@json-render/react";
+import { Component, type ReactNode } from "react";
 import type { VisualizationSpec } from "@/lib/catalog";
 import { registry } from "@/lib/registry";
 
+// Specs render as soon as they finish streaming — before the tool's
+// validation result lands — so a bad spec must degrade to an inline
+// message rather than crash the chat.
+class VisualizationErrorBoundary extends Component<{ children: ReactNode }, { failed: boolean }> {
+  state = { failed: false };
+
+  static getDerivedStateFromError() {
+    return { failed: true };
+  }
+
+  render() {
+    if (this.state.failed) {
+      return (
+        <div className="my-3 rounded-lg border border-dashed px-3 py-2 text-xs text-muted-foreground">
+          Couldn&apos;t render this visualization.
+        </div>
+      );
+    }
+    return this.props.children;
+  }
+}
+
 export function Visualization({ spec }: { spec: VisualizationSpec }) {
   return (
     <div className="my-3">
-      <JSONUIProvider registry={registry}>
-        <Renderer spec={spec} registry={registry} />
-      </JSONUIProvider>
+      <VisualizationErrorBoundary>
+        <JSONUIProvider registry={registry}>
+          <Renderer spec={spec} registry={registry} />
+        </JSONUIProvider>
+      </VisualizationErrorBoundary>
     </div>
   );
 }

clickhouse-chat-agent/src/lib/catalog.ts

@@ -15,11 +15,14 @@ const chartData = z
   .array(z.record(z.string(), z.union([z.string(), z.number(), z.null()])))
   .describe("Data rows, one object per x-axis entry");
 
+// Nested optional fields use .nullish() (not .nullable()) so the model can
+// omit them: the top-level null-fill in validateSpec doesn't recurse into
+// arrays, and bare .nullable() rejects a missing key.
 const series = z
   .array(
     z.object({
       dataKey: z.string().describe("Key in each data row holding this series' numeric value"),
-      label: z.string().nullable().describe("Human-readable series name for legend/tooltip"),
+      label: z.string().nullish().describe("Human-readable series name for legend/tooltip"),
     })
   )
   .describe("One entry per plotted series");
@@ -74,10 +77,10 @@ export const chartComponentDefinitions = {
           z.object({
             lat: z.number(),
             lng: z.number(),
-            label: z.string().nullable().describe("Shown in the marker tooltip"),
+            label: z.string().nullish().describe("Shown in the marker tooltip"),
             value: z
               .number()
-              .nullable()
+              .nullish()
               .describe("Optional magnitude — scales the marker size and shows in the tooltip"),
           })
         )