Dashboard design improvements (#42)

Author: notSumit25

agent/skills/dashboard-design/SKILL.md

@@ -1,7 +1,7 @@
 ---
 name: dashboard-design
 description: Design and code a self-contained HTML dashboard for DeepSQL — ground on the schema, verify SQL, then write a beautiful single-file dashboard that loads data via the deepsql.query bridge.
-version: 2.0.0
+version: 2.1.0
 platforms: [linux, macos, windows]
 metadata:
   hermes:
@@ -23,16 +23,26 @@ await deepsql.query("SELECT ...")         // -> { columns: string[], rows: any[]
 deepsql.ready(fn)                          // runs fn() once the bridge is live (use this to kick off loading)
 
 // Charts — ALWAYS use these instead of hand-writing SVG. Built-in hover tooltips
-// (show the value on mouse-over), number formatting, sparse axis labels, and a
-// graceful "No data" empty state. Pass the deepsql.query result straight in, or
-// [{label,value}] / [[label,value]]. First column = label, second = value (or
-// opts.labelKey/valueKey). opts: { valueFormat(fn), height, color, emptyText }.
+// (show the value on mouse-over), number formatting, sparse axis labels, a
+// graceful "No data" empty state, and a corner expand button that opens the
+// same chart larger in an overlay — all automatic, nothing to wire yourself.
+// Pass the deepsql.query result straight in, or [{label,value}] / [[label,value]].
+// First column = label, second = value (or opts.labelKey/valueKey).
+// opts: { valueFormat(fn), height, color, emptyText, title }.
 deepsql.charts.bar(elOrSelector, data, opts)     // rankings, counts by day
 deepsql.charts.line(elOrSelector, data, opts)     // trends over time (area+line)
 deepsql.charts.donut(elOrSelector, data, opts)    // share/composition (with legend + %)
 deepsql.charts.format(n)                           // human number formatter
 ```
 
+**Chart sizing and the expand control are handled by the runtime — do not build your own.**
+Height is fixed regardless of container width (a chart in a wide card never balloons), and every
+chart already gets a corner "expand" button that opens a larger re-render in an overlay — this is
+exactly the kind of per-chart chrome that's tempting to hand-roll and easy to get inconsistent
+across widgets, so it's built into `deepsql.charts.*` once instead. Pass `opts.title` (the chart's
+plain-business-language heading) so the expanded overlay has something to show as its title — do
+not add your own zoom/expand/fullscreen button, modal, or lightbox; one already exists per chart.
+
 Hard rules:
 - Inline everything — one `<style>`, one or more `<script>`. **No external URLs, CDNs, fonts, or images** (blocked by CSP) and **no `fetch()`/XHR/WebSocket** — data comes only from `deepsql.query`.
 - Never hardcode result data. Query live on load, and re-query when a control changes.
@@ -52,6 +62,9 @@ Hard rules:
    - No `undefined` / `null` / `NaN` can reach the screen — every injected value is guarded with a fallback. Pay special attention to KPI sub-labels and any computed % (e.g. a "top source share" caption).
    - Every explicit user ask from the intent checklist is present and wired (controls default correctly and re-query on change).
    - No table/column/SQL/connection-id text is visible anywhere.
+   - No AI-slop pattern from the section below is present (gradient background/hero, emoji-as-icon,
+     decorative blobs/glassmorphism, uniform shadows, off-scale spacing/type, more than one accented
+     "hero" card, hand-rolled chart colors or expand/zoom controls).
    A dashboard that renders with a blank chart or an "undefined" label is a failed build — catch it here.
 
 ## NEVER expose internals (security + UX — non-negotiable)
@@ -80,6 +93,45 @@ Rules:
 - Numbers formatted for humans (`deepsql.charts.format(n)` / thousands separators; currency symbol from the business rule) — never raw.
 - **Never render `undefined`, `null`, or `NaN`.** Guard every value you inject into the DOM (`v == null ? '—' : v`); a KPI sub-line/label with no value must fall back to a dash or be omitted — not the literal text "undefined".
 
+## Avoid AI-slop patterns (named, so you can catch yourself)
+
+These are the specific tells that make a generated dashboard look generated instead of
+designed. Each one is easy to reach for by default — that's exactly why it needs to be named
+and ruled out explicitly, not left to taste.
+
+- **No default purple/blue/pink gradient backgrounds.** A `linear-gradient(135deg, #667eea, #764ba2)`-style
+  hero band, header, or card is the single most recognizable AI-generated-UI signature. `--ds-grad`
+  exists for exactly one purpose — a subtle lift on the single most important KPI card — never a page
+  background, never a header banner, never more than one card on the whole dashboard.
+- **No emoji as icons, bullets, or section markers.** Not in KPI labels, not in section headers, not
+  as a substitute for a real icon. If a visual marker is needed, use a plain shape (a dot, a small
+  colored square in a legend) — never 📊📈💰✨ etc.
+- **No oversized rounded "blob" shapes, decorative background circles, or glassmorphism for its own
+  sake.** `backdrop-filter`/translucency is not part of this theme — don't add it. Every visual
+  element must carry information (a card, a chart, a legend swatch); nothing is decoration.
+- **No uniform drop-shadow on every element.** `--ds-shadow` is for cards that sit on `--ds-bg` —
+  don't add extra shadows to buttons, badges, or text, and don't stack multiple shadow layers for
+  "depth." Flat and quiet is correct here.
+- **No arbitrary one-off spacing or font sizes.** Pick from a small fixed scale and stay on it for
+  the whole document:
+  - Spacing: `4px 8px 12px 16px 24px 32px` — nothing between these, nothing larger without a real reason.
+  - Type: 3 sizes total — a KPI number (~28–32px, bold), section/card headings (~14–15px, semibold),
+    body/labels (~12–13px, regular). Don't introduce a fourth size for a one-off caption.
+- **No centered "hero" layout with everything stacked in one narrow column.** This is a working
+  dashboard, not a landing page — use a real grid (`auto-fit`/`auto-fill` KPI row, multi-column chart
+  layout) that uses the available width purposefully.
+- **Chart color discipline:** stick to the theme's own greyscale chart palette (already built into
+  `deepsql.charts.*` — you don't choose chart colors). Don't override `opts.color` per chart to
+  introduce your own arbitrary hues; the built-in palette is the whole point of using the shared
+  chart runtime instead of hand-rolled SVG.
+- **One hero KPI, not a "hero row."** If more than one card gets the gradient/accent treatment,
+  none of them read as important — that defeats the point. Pick the single number the business
+  question is actually about and reserve the accent for it alone.
+
+Before emitting, ask: **would this ship, unedited, from a design team that obsesses over every
+pixel — or does it look like the first thing a template generator produced?** If any of the
+patterns above are present, that's your answer.
+
 ## Interaction
 
 - Wire controls to re-run only the affected queries and re-render — never reload the page. A date range picker defaults to what the user asked for (e.g. today) and drives every time-sensitive query.

backend/src/main/java/com/dbaagent/service/DashboardAgentService.java

@@ -5,6 +5,11 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.ai.chat.client.ChatClient;
+import org.springframework.ai.chat.messages.Message;
+import org.springframework.ai.chat.messages.SystemMessage;
+import org.springframework.ai.chat.messages.UserMessage;
+import org.springframework.ai.chat.model.ChatModel;
 import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Service;
 import org.springframework.web.server.ResponseStatusException;
@@ -51,15 +56,18 @@ public interface StepListener {
     private final AccessControlService accessControlService;
     private final AgentBridgeService agentBridgeService;
     private final AgentChatClient agentChatClient;
+    private final ChatClient intentChatClient;
 
     public DashboardAgentService(ObjectMapper objectMapper,
                                  AccessControlService accessControlService,
                                  AgentBridgeService agentBridgeService,
-                                 AgentChatClient agentChatClient) {
+                                 AgentChatClient agentChatClient,
+                                 ChatModel chatModel) {
         this.objectMapper = objectMapper;
         this.accessControlService = accessControlService;
         this.agentBridgeService = agentBridgeService;
         this.agentChatClient = agentChatClient;
+        this.intentChatClient = ChatClient.builder(chatModel).build();
     }
 
     public Map<String, Object> generate(String connectionId, String prompt, Object currentConfig, StepListener listener) {
@@ -75,6 +83,26 @@ public Map<String, Object> generate(String connectionId, String prompt, Object c
             throw new ResponseStatusException(HttpStatus.BAD_GATEWAY, "The DeepSQL agent is unavailable right now.");
         }
 
+        // Most messages are a real build/edit ask, so default to the full pipeline —
+        // only skip it for something that plainly isn't one (a greeting, a question
+        // about the tool itself). Answering "hi" by grounding on the schema, writing
+        // SQL, and self-reviewing an HTML document is where the multi-minute replies
+        // to trivial messages came from.
+        if (isChatOnly(prompt)) {
+            emit(l, trace, "planning", "Replying…");
+            AgentChatClient.AgentReply chatReply = agentChatClient.sendAndAwait(sessionId, buildChatTask(prompt));
+            if (chatReply.ok() && chatReply.text() != null && !chatReply.text().isBlank()) {
+                emit(l, trace, "done", "Replied");
+                Map<String, Object> chat = new LinkedHashMap<>();
+                chat.put("chat", true);
+                chat.put("reply", chatReply.text().trim());
+                chat.put("trace", trace);
+                return chat;
+            }
+            // Ambiguous or the agent didn't just answer — fall through to a real build
+            // rather than surfacing a failure for what might be a legitimate request.
+        }
+
         emit(l, trace, "planning", "Agent is grounding, writing SQL, and coding the dashboard…");
         AgentChatClient.AgentReply reply = agentChatClient.sendAndAwait(
             sessionId, buildTask(connectionId, prompt, currentConfig));
@@ -106,6 +134,51 @@ public Map<String, Object> generate(String connectionId, String prompt, Object c
         return cfg;
     }
 
+    // ── chat-only detection ─────────────────────────────────────────────────
+
+    // A one-word classification call, not a keyword match: a fixed word list can't
+    // tell "make it prettier" or "no, the other one" from a real edit ask. This is a
+    // direct ChatModel call (no agent session, no tools) so it stays fast — the whole
+    // point is answering "hi" without paying for a grounding+SQL+self-review turn.
+    // Biased toward CHAT=false (i.e. toward the full pipeline) in the prompt itself:
+    // a wrong "this is chat" guess on a real request is far worse than an occasional
+    // unnecessary grounding pass on a genuine one-word greeting.
+    private static final String INTENT_SYSTEM_PROMPT = """
+        Classify one chat message from a BI dashboard builder. Decide whether it is a
+        request to build, edit, or change a chart/dashboard/metric/data view (CHAT=false),
+        or plainly just conversation — a greeting, thanks, or a question about the tool
+        itself with no dashboard content in it (CHAT=true).
+
+        If in doubt, answer false — treat anything that could plausibly be about the data
+        or the dashboard's content/appearance as a real request, even if short or vague
+        ("make it prettier", "no, the other one", "add a filter").
+
+        Reply with exactly one word, "true" or "false". No punctuation, no explanation.
+        """;
+    private static final int CHAT_ONLY_MAX_CHARS = 200;
+
+    private boolean isChatOnly(String prompt) {
+        if (prompt == null) return false;
+        String p = prompt.trim();
+        if (p.isEmpty() || p.length() > CHAT_ONLY_MAX_CHARS) return false;
+        try {
+            List<Message> messages = List.of(new SystemMessage(INTENT_SYSTEM_PROMPT), new UserMessage(p));
+            String verdict = intentChatClient.prompt().messages(messages).call().content();
+            return verdict != null && verdict.trim().toLowerCase().startsWith("true");
+        } catch (Exception e) {
+            log.warn("Chat-intent classification failed, defaulting to full pipeline: {}", e.getMessage());
+            return false;
+        }
+    }
+
+    private String buildChatTask(String prompt) {
+        return "The user sent this message in the dashboard builder's chat: \"" + prompt.trim() + "\"\n\n"
+            + "It does not read as a request to build or change a chart/dashboard — it looks like a "
+            + "greeting, small talk, or a question about what you can do. Reply briefly and naturally "
+            + "in plain text (no HTML, no code block, no tool calls, no grounding, no SQL). If it's a "
+            + "greeting, greet back and invite them to describe a dashboard. Keep it to 1-2 sentences.";
+    }
+
     // ── the task the agent runs ────────────────────────────────────────────
 
     private String buildTask(String connectionId, String prompt, Object currentConfig) {

src/components/DashboardArtifact.jsx

@@ -33,15 +33,25 @@ const BRIDGE = `
     if (d.error) p.reject(new Error(d.error));
     else p.resolve({ columns: d.columns || [], rows: d.rows || [] });
   });
+  // Report only the content's own height, never the iframe's current rendered
+  // height — scrollHeight on a body with height:auto reflects content size and
+  // can't be inflated by whatever height the parent last set, so this can't
+  // feed back into itself. Debounced and deduped so parent-side layout thrash
+  // (e.g. a page scroll) can't retrigger it with the same value.
+  var lastReported=-1, reportTimer=null;
   function reportHeight(){
-    var h = Math.max(document.body ? document.body.scrollHeight : 0,
-                     document.documentElement ? document.documentElement.scrollHeight : 0);
-    send({ __deepsql:true, type:'height', value: h });
+    if (reportTimer) return;
+    reportTimer = setTimeout(function(){
+      reportTimer = null;
+      var h = document.documentElement ? document.documentElement.scrollHeight : 0;
+      if (h === lastReported) return;
+      lastReported = h;
+      send({ __deepsql:true, type:'height', value: h });
+    }, 50);
   }
   window.addEventListener('load', function(){
     reportHeight();
     try { new ResizeObserver(reportHeight).observe(document.body); } catch(e){}
-    setInterval(reportHeight, 1000);
   });
   window.addEventListener('error', function(e){
     send({ __deepsql:true, type:'jserror', message: (e && e.message) || 'script error' });
@@ -84,9 +94,13 @@ const MAX_TOTAL_QUERIES = 400
 const QUERY_TIMEOUT_MS = 25000
 const sleep = (ms) => new Promise((r) => setTimeout(r, ms))
 
+const REDUCED_MOTION = typeof window !== 'undefined'
+  && window.matchMedia?.('(prefers-reduced-motion: reduce)').matches
+
 export default function DashboardArtifact({ connectionId, html, onError, queryFn }) {
   const iframeRef = useRef(null)
   const [height, setHeight] = useState(600)
+  const [loaded, setLoaded] = useState(false)
   const queueRef = useRef([])
   const inflightRef = useRef(0)
   const totalRef = useRef(0)
@@ -162,11 +176,13 @@ export default function DashboardArtifact({ connectionId, html, onError, queryFn
   }, [onMessage])
 
   // New artifact (generate/edit) reloads the iframe — reset the throttle so a
-  // fresh dashboard isn't blocked by the prior one's runaway cap.
+  // fresh dashboard isn't blocked by the prior one's runaway cap, and fade the
+  // new one in rather than popping at whatever height it first reports.
   useEffect(() => {
     queueRef.current = []
     inflightRef.current = 0
     totalRef.current = 0
+    setLoaded(false)
   }, [html])
 
   return (
@@ -175,7 +191,19 @@ export default function DashboardArtifact({ connectionId, html, onError, queryFn
       title="Dashboard"
       sandbox="allow-scripts"
       srcDoc={buildSrcDoc(html || '', connectionId)}
-      style={{ width: '100%', height, border: 'none', display: 'block', background: '#f8fafc' }}
+      onLoad={() => setLoaded(true)}
+      style={{
+        width: '100%',
+        height,
+        border: 'none',
+        display: 'block',
+        background: '#f8fafc',
+        opacity: loaded ? 1 : 0,
+        // Height snaps immediately, never transitions — animating it risked measuring
+        // the iframe's own in-transition rendered height as if it were new content,
+        // a feedback loop that made the page grow on every scroll/resize tick.
+        transition: REDUCED_MOTION ? 'opacity 150ms linear' : 'opacity 320ms cubic-bezier(0.2, 0.8, 0.2, 1)',
+      }}
     />
   )
 }