Chart builder: data-aware defaults, one-click hint fixes, canvas preview, fullscreen modal

src/core/chart-builder.test.ts

@@ -108,16 +108,20 @@ describe('builderWarnings (Tier B advisories)', () => {
     expect(w.some((m) => /need both an X and a Y/.test(m.message))).toBe(true);
   });
 
-  it('warns when two measures are drawn on a non-scatter mark', () => {
-    const w = builderWarnings({
+  it('warns when two measures are drawn on a non-scatter mark, offering [Switch to Point]', () => {
+    const config: BuilderConfig = {
       datasetName: 'D',
       mark: 'bar',
       encodings: {
         x: { field: 'a', type: 'quantitative' },
         y: { field: 'b', type: 'quantitative' },
       },
-    });
-    expect(w.some((m) => /scatter/.test(m.message))).toBe(true);
+    };
+    const w = builderWarnings(config);
+    const hint = w.find((m) => /scatter/.test(m.message));
+    const fix = hint?.fixes?.find((f) => f.label === 'Switch to Point');
+    expect(fix).toBeDefined();
+    expect(fix!.apply(config).mark).toBe('point');
   });
 
   it('warns when a bar/line/area has no measure on either axis', () => {
@@ -129,8 +133,8 @@ describe('builderWarnings (Tier B advisories)', () => {
     expect(w.some((m) => /need a measure/.test(m.message))).toBe(true);
   });
 
-  it('warns when an area chart is split into colour series', () => {
-    const w = builderWarnings({
+  it('warns when an area chart is split into colour series, offering [Stack] / [Remove colour]', () => {
+    const config: BuilderConfig = {
       datasetName: 'D',
       mark: 'area',
       encodings: {
@@ -138,8 +142,20 @@ describe('builderWarnings (Tier B advisories)', () => {
         y: { field: 'v', type: 'quantitative' },
         color: { field: 'g', type: 'nominal' },
       },
-    });
-    expect(w.some((m) => m.channel === 'color')).toBe(true);
+    };
+    const w = builderWarnings(config);
+    const hint = w.find((m) => m.channel === 'color');
+    expect(hint).toBeDefined();
+    const labels = hint?.fixes?.map((f) => f.label) ?? [];
+    expect(labels).toEqual(['Stack', 'Remove colour']); // most-recommended first
+    // [Remove colour] clears the colour channel, so the hint re-derives away.
+    const cleared = hint!.fixes!.find((f) => f.label === 'Remove colour')!.apply(config);
+    expect(cleared.encodings.color).toBeNull();
+    expect(builderWarnings(cleared).some((m) => m.channel === 'color')).toBe(false);
+    // [Stack] turns it into a part-to-whole stack, which is no longer flagged.
+    const stacked = hint!.fixes!.find((f) => f.label === 'Stack')!.apply(config);
+    expect(stacked.stack).toBe('zero');
+    expect(builderWarnings(stacked).some((m) => m.channel === 'color')).toBe(false);
   });
 
   it('is silent for a clean configuration', () => {
@@ -170,7 +186,29 @@ describe('builderWarnings (Tier B advisories)', () => {
       const hint = w.find((m) => /one mark per row/.test(m.message));
       expect(hint?.channel).toBe('x'); // the category axis
       expect(hint?.message).toContain('406 in this dataset');
-      expect(hint?.message).toMatch(/Swap X\/Y/); // bar → horizontal-bar remedy
+      // Remedies are one-click fixes, not prose; most-recommended first.
+      const labels = hint?.fixes?.map((f) => f.label) ?? [];
+      expect(labels).toEqual(['Aggregate as Sum', 'Swap X/Y']); // aggregate before swap
+    });
+
+    it('[Aggregate as Sum] resolves the one-mark-per-row hint', () => {
+      const w = crowded();
+      const hint = w.find((m) => /one mark per row/.test(m.message));
+      const fix = hint?.fixes?.find((f) => f.label === 'Aggregate as Sum');
+      expect(fix).toBeDefined();
+      const fixed = fix!.apply({
+        datasetName: 'D',
+        mark: 'bar',
+        encodings: {
+          x: { field: 'name', type: 'nominal' },
+          y: { field: 'mpg', type: 'quantitative' },
+        },
+      });
+      expect(fixed.encodings.y?.aggregate).toBe('sum');
+      // and the hint is gone once applied
+      expect(builderWarnings(fixed, 406).some((m) => /one mark per row/.test(m.message))).toBe(
+        false,
+      );
     });
 
     it('is silent once the measure is aggregated (one bar per category)', () => {
@@ -219,8 +257,9 @@ describe('builderWarnings (Tier B advisories)', () => {
       );
       const hint = w.find((m) => /one mark per row/.test(m.message));
       expect(hint).toBeDefined();
-      expect(hint?.message).not.toMatch(/Swap X\/Y/);
-      expect(hint?.message).toMatch(/reduce the number of categories/);
+      const labels = hint?.fixes?.map((f) => f.label) ?? [];
+      expect(labels).not.toContain('Swap X/Y'); // a horizontal line makes no sense
+      expect(labels).toContain('Aggregate as Sum'); // aggregate still applies
     });
   });
 
@@ -262,7 +301,7 @@ describe('builderWarnings (Tier B advisories)', () => {
       const hint = w.find((m) => /distinct values/.test(m.message));
       expect(hint?.channel).toBe('x');
       expect(hint?.message).toMatch(/48 distinct values/);
-      expect(hint?.message).toMatch(/Swap X\/Y/); // bar → horizontal-bar remedy
+      expect(hint?.fixes?.map((f) => f.label)).toContain('Swap X/Y'); // horizontal-bar remedy
     });
 
     it('reports "more than 50" when the category cardinality hit the profiler cap', () => {
@@ -426,7 +465,8 @@ describe('builderWarnings (Tier B advisories)', () => {
 });
 
 describe('defaultBuilderConfig', () => {
-  it('puts the first column on X and the second on Y, each with derived type', () => {
+  it('falls back to first-on-X, second-on-Y when the dataset is unprofiled', () => {
+    // `columns` carries no columnStats → no data-aware pick → positional default.
     const config = defaultBuilderConfig('Sales', columns);
     expect(config.mark).toBe('bar');
     expect(config.datasetName).toBe('Sales');
@@ -467,6 +507,91 @@ describe('defaultBuilderConfig', () => {
   });
 });
 
+describe('defaultBuilderConfig — data-aware "safest bet" (profiled datasets)', () => {
+  /** Stats for one column. */
+  const stat = (name: string, distinct: number, capped = false) => ({
+    name,
+    distinct,
+    distinctCapped: capped,
+    numericExtent: null,
+  });
+
+  it('opens on a low-cardinality category vs a count of records, not the first two columns', () => {
+    // Superstore-shaped: an id-like number first, a high-cardinality id, then tidy
+    // categories — the case a positional first-two-columns default would open as a
+    // 9994-bar degenerate chart.
+    const wide: BuilderColumns = {
+      columns: ['Row ID', 'Order ID', 'Segment', 'Sales'],
+      columnTypes: [
+        { name: 'Row ID', type: 'number' },
+        { name: 'Order ID', type: 'string' },
+        { name: 'Segment', type: 'string' },
+        { name: 'Sales', type: 'number' },
+      ],
+      columnStats: [
+        stat('Row ID', 50, true),
+        stat('Order ID', 50, true), // high cardinality → not a category axis
+        stat('Segment', 3), // tidy category → the pick
+        stat('Sales', 50, true),
+      ],
+    };
+    const config = defaultBuilderConfig('Superstore', wide);
+    expect(config.mark).toBe('bar');
+    expect(config.encodings.x).toEqual({ field: 'Segment', type: 'nominal' });
+    expect(config.encodings.y).toEqual({ type: 'quantitative', aggregate: 'count' });
+  });
+
+  it('picks the lowest-cardinality readable category among several', () => {
+    const cols: BuilderColumns = {
+      columns: ['Region', 'Segment', 'City'],
+      columnTypes: [
+        { name: 'Region', type: 'string' },
+        { name: 'Segment', type: 'string' },
+        { name: 'City', type: 'string' },
+      ],
+      columnStats: [stat('Region', 4), stat('Segment', 3), stat('City', 50, true)],
+    };
+    const config = defaultBuilderConfig('D', cols);
+    expect(config.encodings.x).toEqual({ field: 'Segment', type: 'nominal' }); // 3 < 4
+  });
+
+  it('falls through to a time series (date vs count) when no tidy category exists', () => {
+    const cols: BuilderColumns = {
+      columns: ['Order ID', 'Order Date', 'Sales'],
+      columnTypes: [
+        { name: 'Order ID', type: 'string' },
+        { name: 'Order Date', type: 'date' },
+        { name: 'Sales', type: 'number' },
+      ],
+      columnStats: [
+        stat('Order ID', 50, true),
+        stat('Order Date', 50, true),
+        stat('Sales', 50, true),
+      ],
+    };
+    const config = defaultBuilderConfig('D', cols);
+    expect(config.mark).toBe('line');
+    expect(config.encodings.x).toEqual({ field: 'Order Date', type: 'temporal' });
+    expect(config.encodings.y).toEqual({ field: 'Sales', type: 'quantitative' }); // the measure, raw
+  });
+
+  it('falls through to a scatter of two measures when there is no category or date', () => {
+    const cols: BuilderColumns = {
+      columns: ['Order ID', 'Sales', 'Profit'],
+      columnTypes: [
+        { name: 'Order ID', type: 'string' },
+        { name: 'Sales', type: 'number' },
+        { name: 'Profit', type: 'number' },
+      ],
+      columnStats: [stat('Order ID', 50, true), stat('Sales', 50, true), stat('Profit', 50, true)],
+    };
+    const config = defaultBuilderConfig('D', cols);
+    expect(config.mark).toBe('point');
+    expect(config.encodings.x).toEqual({ field: 'Sales', type: 'quantitative' });
+    expect(config.encodings.y).toEqual({ field: 'Profit', type: 'quantitative' });
+  });
+});
+
 describe('isBuilderConfigValid', () => {
   const base: BuilderConfig = { datasetName: 'D', mark: 'bar', encodings: {} };
 

src/core/chart-builder.ts

@@ -230,14 +230,87 @@ function fieldTypeForColumn(name: string, columns: BuilderColumns): FieldType {
   return defaultFieldType(match?.type ?? 'string');
 }
 
+/**
+ * Above this many distinct values, a column is too high-cardinality to be a good
+ * default category axis: its labels overlap into an unreadable axis, and an
+ * unaggregated chart that wide can exceed the canvas size limit. Matches the
+ * crowded-axis warning threshold.
+ */
+const CATEGORY_DEFAULT_MAX_DISTINCT = 30;
+
+/**
+ * Pick a *sensible, renderable* default X/Y from the data shape, using profiled
+ * cardinality (`columnStats`). This avoids opening the builder on a degenerate
+ * one-mark-per-row chart — the shape a positional first-two-columns rule yields when
+ * the leading columns are an id and a high-cardinality key. Returns `null` when there
+ * are no stats to reason about (older / URL datasets), so the caller falls back to the
+ * positional default.
+ *
+ * Preference order (each guaranteed to render and read cleanly):
+ *   1. a low-cardinality **category** vs a **count of records** → a tidy bar;
+ *   2. else a **temporal** axis vs count → a time series (continuous x, always fits);
+ *   3. else two **measures** → a scatter (continuous axes, always fit).
+ *
+ * The category case pairs with the field-less **count**, not a raw measure: count is
+ * always meaningful and avoids summing an id-like numeric (Row ID, Postal Code) into
+ * nonsense. This is the builder's *opening* state only; a later intent-first entry
+ * point can layer richer recommendations on top.
+ */
+function smartDefaultEncodings(
+  columns: BuilderColumns,
+): { x: ChannelMapping; y: ChannelMapping } | null {
+  const stats = columns.columnStats;
+  if (!stats || stats.length === 0) return null; // no profiling → positional fallback
+  const typeOf = (name: string): ColumnType =>
+    columns.columnTypes.find((c) => c.name === name)?.type ?? 'string';
+  const knownDistinct = (name: string): number | undefined => {
+    const s = stats.find((x) => x.name === name);
+    return s && !s.distinctCapped ? s.distinct : undefined;
+  };
+  const count: ChannelMapping = { type: 'quantitative', aggregate: 'count' };
+  const numbers = columns.columns.filter((name) => typeOf(name) === 'number');
+
+  // 1. The lowest-cardinality readable category (string/boolean) → a tidy bar. Pair it
+  //    with **count** (not a raw measure): a raw measure would draw one bar per row.
+  const category = columns.columns
+    .filter((name) => {
+      const t = typeOf(name);
+      if (t !== 'string' && t !== 'boolean') return false;
+      const d = knownDistinct(name);
+      return d !== undefined && d >= 2 && d <= CATEGORY_DEFAULT_MAX_DISTINCT;
+    })
+    .sort((a, b) => (knownDistinct(a) ?? 0) - (knownDistinct(b) ?? 0))[0];
+  if (category) return { x: { field: category, type: 'nominal' }, y: count };
+
+  // 2. A date → a time series of the first measure (a temporal axis is continuous, so a
+  //    line of raw values always fits); fall back to count if there is no measure.
+  const temporal = columns.columns.find((name) => typeOf(name) === 'date');
+  if (temporal) {
+    const y: ChannelMapping = numbers[0] ? { field: numbers[0], type: 'quantitative' } : count;
+    return { x: { field: temporal, type: 'temporal' }, y };
+  }
+
+  // 3. Two measures → a scatter (continuous axes, always renderable).
+  if (numbers.length >= 2) {
+    return {
+      x: { field: numbers[0], type: 'quantitative' },
+      y: { field: numbers[1], type: 'quantitative' },
+    };
+  }
+  return null;
+}
+
 /**
  * The builder's opening configuration for a dataset (spec §06 → Default
- * pre-population, Tier B): the first column on X and the second (if any) on Y, each
- * with its derived field type; Color and Size start unmapped, no transforms. The
- * mark is the **smart default** for the resulting X/Y shape (`defaultMark`) rather
- * than always Bar — a date-vs-number dataset opens as a Line, two measures as a
- * Point — so the first preview is already the conventional chart. A dataset with no
- * detected columns yields an all-unmapped config (the modal then prompts).
+ * pre-population, Tier B). When the dataset is profiled, X/Y are chosen as a
+ * **data-aware "safest bet"** (`smartDefaultEncodings`) — a low-cardinality category
+ * vs a count of records (a tidy bar), else a time series, else a scatter — so the
+ * builder never opens on a degenerate one-mark-per-row chart that can't render. When
+ * there are no stats to reason about, it falls back to the positional rule: first
+ * column on X, second (if any) on Y, each with its derived field type. Either way the
+ * mark is the **smart default** for the resulting X/Y shape (`defaultMark`), Color
+ * and Size start unmapped with no transforms, and a dataset with no detected columns
+ * yields an all-unmapped config (the modal then prompts).
  */
 export function defaultBuilderConfig(datasetName: string, columns: BuilderColumns): BuilderConfig {
   const encodings: Partial<Record<ChannelName, ChannelMapping | null>> = {
@@ -246,12 +319,20 @@ export function defaultBuilderConfig(datasetName: string, columns: BuilderColumn
     color: null,
     size: null,
   };
-  const [first, second] = columns.columns;
-  if (first !== undefined) {
-    encodings.x = { field: first, type: fieldTypeForColumn(first, columns) };
-  }
-  if (second !== undefined) {
-    encodings.y = { field: second, type: fieldTypeForColumn(second, columns) };
+  // Prefer a data-aware "safest bet" (a renderable, readable chart) when the dataset
+  // is profiled; otherwise fall back to the positional first-on-X, second-on-Y rule.
+  const smart = smartDefaultEncodings(columns);
+  if (smart) {
+    encodings.x = smart.x;
+    encodings.y = smart.y;
+  } else {
+    const [first, second] = columns.columns;
+    if (first !== undefined) {
+      encodings.x = { field: first, type: fieldTypeForColumn(first, columns) };
+    }
+    if (second !== undefined) {
+      encodings.y = { field: second, type: fieldTypeForColumn(second, columns) };
+    }
   }
   const mark = defaultMark(encodings.x?.type ?? null, encodings.y?.type ?? null);
   return { datasetName, mark, encodings };
@@ -329,12 +410,27 @@ export function supportsStack(config: BuilderConfig): boolean {
   );
 }
 
+/**
+ * A one-click remedy a warning can offer. `apply` is a pure config→config transform;
+ * the modal renders `label` as a button that runs it. It is an *offer*, never a forced
+ * change — once applied, the warning re-derives away. Lives in core so the remedies
+ * unit-test alongside the warnings.
+ */
+export interface BuilderWarningFix {
+  /** The button label naming the remedy, e.g. "Aggregate as Sum". */
+  label: string;
+  /** Produce the corrected configuration from the current one (pure). */
+  apply: (config: BuilderConfig) => BuilderConfig;
+}
+
 /** A non-blocking advisory about a configuration (spec §06 → Tier B warnings). */
 export interface BuilderWarning {
   /** The channel the hint is about, when it's channel-specific. */
   channel?: ChannelName;
   /** A short, plain-language hint the modal shows inline (not an error). */
   message: string;
+  /** Optional one-click remedies the modal renders as buttons next to the hint. */
+  fixes?: BuilderWarningFix[];
 }
 
 /**
@@ -371,6 +467,41 @@ function cardinalityText(stats: ColumnStats): string {
   return stats.distinctCapped ? `more than ${DISTINCT_CAP}` : `${stats.distinct}`;
 }
 
+// --- Pure config transforms backing the actionable-hint fixes. They mirror the
+// store's setChannelAggregate / swapXY / setStack / setChannelColumn(null) / setMark
+// actions, so applying a fix and making the equivalent manual edit land on the same
+// config. Kept here (not the store) so the remedies are pure and unit-testable. ---
+
+/** Aggregate one channel's field (clearing any bin — the two are mutually exclusive). */
+function withChannelAggregate(
+  config: BuilderConfig,
+  channel: ChannelName,
+  aggregate: AggregateOp,
+): BuilderConfig {
+  const current = config.encodings[channel];
+  if (!current) return config;
+  const next: ChannelMapping = { ...current, aggregate };
+  delete next.bin;
+  return { ...config, encodings: { ...config.encodings, [channel]: next } };
+}
+
+/** Exchange the X and Y mappings (the manual Swap X/Y, as a pure transform). */
+function withSwappedXY(config: BuilderConfig): BuilderConfig {
+  return {
+    ...config,
+    encodings: {
+      ...config.encodings,
+      x: config.encodings.y ?? null,
+      y: config.encodings.x ?? null,
+    },
+  };
+}
+
+/** Clear one channel back to "None" (drops its mapping from the spec). */
+function withChannelCleared(config: BuilderConfig, channel: ChannelName): BuilderConfig {
+  return { ...config, encodings: { ...config.encodings, [channel]: null } };
+}
+
 /**
  * Non-blocking advisories for the current configuration (spec §06 → Tier B): the
  * encodings that render but read poorly, drawn from the research's soft rules
@@ -427,17 +558,25 @@ export function builderWarnings(
     mark !== 'circle'
   ) {
     warnings.push({
-      message: 'Two measures usually read best as a scatter — try Point or Circle.',
+      message: 'Two measures usually read best as a scatter.',
+      fixes: [{ label: 'Switch to Point', apply: (c) => ({ ...c, mark: 'point' }) }],
     });
   }
 
   // Area split into many series hides per-component change (FT Visual Vocabulary:
   // "seeing change in components can be very difficult").
-  if (mark === 'area' && config.encodings.color) {
+  // (Stacking turns overlapping series into a cumulative part-to-whole, a valid read,
+  // so a stacked area is not flagged — applying the [Stack] fix below clears this.)
+  if (mark === 'area' && config.encodings.color && !config.stack) {
+    const fixes: BuilderWarningFix[] = [];
+    if (supportsStack(config)) {
+      fixes.push({ label: 'Stack', apply: (c) => ({ ...c, stack: 'zero' }) });
+    }
+    fixes.push({ label: 'Remove colour', apply: (c) => withChannelCleared(c, 'color') });
     warnings.push({
       channel: 'color',
-      message:
-        'Area charts make per-series change hard to read; consider Line for multiple series.',
+      message: 'Area charts make per-series change hard to read.',
+      fixes,
     });
   }
 
@@ -455,28 +594,38 @@ export function builderWarnings(
   // long category lists belong on a horizontal bar).
   if (mark === 'bar' || mark === 'line' || mark === 'area') {
     const category = sortableCategoryChannel(config); // discrete axis of a category-vs-measure pair
-    const measure = category ? config.encodings[category === 'x' ? 'y' : 'x'] : null;
-    if (category && measure) {
+    const measureChannel = category ? (category === 'x' ? 'y' : 'x') : null;
+    const measure = measureChannel ? (config.encodings[measureChannel] ?? null) : null;
+    if (category && measureChannel && measure) {
       const rawMeasure = !measure.aggregate && !measure.bin;
       if (rawMeasure && typeof rowCount === 'number' && rowCount > CROWDED_CATEGORY_ROWS) {
-        const fix =
-          mark === 'bar'
-            ? 'Aggregate the measure (e.g. Sum or Mean) for one bar per category, or use Swap X/Y for a horizontal bar where long labels stay readable.'
-            : 'Aggregate the measure (e.g. Sum or Mean) so there is one mark per category, or reduce the number of categories.';
+        // Aggregating the measure collapses one-mark-per-row to one-per-category; a bar
+        // can also flip horizontal (Swap X/Y) where long labels stay readable. Offer
+        // aggregate only for a quantitative measure (Sum is meaningless on a date).
+        const fixes: BuilderWarningFix[] = [];
+        if (effectiveType(measure) === 'quantitative') {
+          fixes.push({
+            label: 'Aggregate as Sum',
+            apply: (c) => withChannelAggregate(c, measureChannel, 'sum'),
+          });
+        }
+        if (mark === 'bar') fixes.push({ label: 'Swap X/Y', apply: withSwappedXY });
         warnings.push({
           channel: category,
-          message: `This draws one mark per row (${rowCount} in this dataset), so the category-axis labels will overlap. ${fix}`,
+          message: `This draws one mark per row (${rowCount} in this dataset), so the category-axis labels will overlap.`,
+          fixes: fixes.length ? fixes : undefined,
         });
       } else {
         const stats = statsFor(config.encodings[category]?.field, columns);
         if (stats && stats.distinct > CROWDED_CATEGORY_DISTINCT) {
-          const fix =
-            mark === 'bar'
-              ? 'Use Swap X/Y for a horizontal bar where long lists stay readable, or filter to fewer categories.'
-              : 'Filter to fewer categories, or group the long tail into an "Other".';
+          // Aggregated already, so the remedy is fewer categories (filter — not yet a
+          // builder control) or, for a bar, a horizontal flip where long lists fit.
+          const fixes: BuilderWarningFix[] =
+            mark === 'bar' ? [{ label: 'Swap X/Y', apply: withSwappedXY }] : [];
           warnings.push({
             channel: category,
-            message: `This category axis has ${cardinalityText(stats)} distinct values, so its labels will overlap. ${fix}`,
+            message: `This category axis has ${cardinalityText(stats)} distinct values, so its labels will overlap.`,
+            fixes: fixes.length ? fixes : undefined,
           });
         }
       }