diff --git a/docs/architecture/06-type-inference.md b/docs/architecture/06-type-inference.md index f4da67b..914b2b5 100644 --- a/docs/architecture/06-type-inference.md +++ b/docs/architecture/06-type-inference.md @@ -169,16 +169,17 @@ A **profile** is the set of derived summary fields stored on a dataset record so the UI can describe it without re-parsing the payload. Per the data model, a profiled dataset carries: -| Field | Type | Meaning | -| ------------- | ----------------------- | ---------------------------------- | -| `rowCount` | `number \| null` | Data rows, or `null` when N/A. | -| `columnCount` | `number \| null` | Columns, or `null` when N/A. | -| `columns` | `string[]` | Column names, in order. | -| `columnTypes` | `Array<{ name; type }>` | Per-column inferred type (see §2). | -| `size` | `number` | Approximate payload size in bytes. | +| Field | Type | Meaning | +| ------------- | ---------------------------------------------------------- | ------------------------------------------------------------------ | +| `rowCount` | `number \| null` | Data rows, or `null` when N/A. | +| `columnCount` | `number \| null` | Columns, or `null` when N/A. | +| `columns` | `string[]` | Column names, in order. | +| `columnTypes` | `Array<{ name; type }>` | Per-column inferred type (see §2). | +| `columnStats` | `Array<{ name; distinct; distinctCapped; numericExtent }>` | Per-column cardinality + numeric range (see §3.3). Empty when N/A. | +| `size` | `number` | Approximate payload size in bytes. | -`null` row/column counts and an empty `columns`/`columnTypes` are how the UI -shows **"N/A"** — see §3.2. +`null` row/column counts and an empty `columns`/`columnTypes`/`columnStats` are +how the UI shows **"N/A"** — see §3.2. ### 3.1 What gets profiled @@ -208,12 +209,14 @@ of the stored payload), but `rowCount` and `columnCount` are `null`, and - `json` that is a non-empty **array of objects** → use it directly. - anything else (`topojson`, a lone JSON object, an empty array) → not tabular; return the N/A profile (`rowCount: null`, `columnCount: null`, - `columns: []`, `columnTypes: []`, plus `size`). + `columns: []`, `columnTypes: []`, `columnStats: []`, plus `size`). 3. **Derive columns** from the union of keys across the rows (or the CSV/TSV header), preserving first-seen order. -4. **Infer each column's type** by collecting that column's values across the - rows and calling `inferColumnType` (§2), sampling per §4. -5. **Assemble** `rowCount`, `columnCount`, `columns`, `columnTypes`, `size`. +4. **Infer each column's type and stats** by collecting that column's values across + the rows (sampling per §4) and calling `inferColumnType` (§2) plus deriving its + cardinality + numeric extent (§3.3) — one pass per column over the same sample. +5. **Assemble** `rowCount`, `columnCount`, `columns`, `columnTypes`, `columnStats`, + `size`. ### Sketch @@ -226,6 +229,12 @@ export interface DatasetProfile { columnCount: number | null; columns: string[]; columnTypes: Array<{ name: string; type: ColumnType }>; + columnStats: Array<{ + name: string; + distinct: number; // capped at DISTINCT_CAP + distinctCapped: boolean; // true ⇒ real cardinality ≥ DISTINCT_CAP + numericExtent: { min: number; max: number } | null; // numeric columns only + }>; size: number; } @@ -234,6 +243,7 @@ const NA = (size: number): DatasetProfile => ({ columnCount: null, columns: [], columnTypes: [], + columnStats: [], size, }); @@ -259,16 +269,22 @@ export function profileData( if (columns.length === 0) return NA(size); const sample = sampleRows(rows); - const columnTypes = columns.map((name) => ({ - name, - type: inferColumnType(sample.map((r) => r[name])), - })); + // One pass per column over the sample: type + stats (cardinality, numeric extent). + const columnTypes = []; + const columnStats = []; + for (const name of columns) { + const values = sample.map((r) => r[name]); + const type = inferColumnType(values); + columnTypes.push({ name, type }); + columnStats.push(columnStatsFor(name, values, type)); // see §3.3 + } return { rowCount: rows.length, columnCount: columns.length, columns, columnTypes, + columnStats, size, }; } @@ -278,17 +294,41 @@ Parsing CSV/TSV text and detecting the payload shape happen _upstream_ of `profileData`; this function takes already-parsed rows so it stays pure and trivially testable. The caller passes `null` for URL and non-tabular datasets. +### 3.3 Column stats: cardinality + numeric extent + +Alongside the display type, each column carries the two data-shape signals the +**Chart Builder** needs for its data-aware Tier-B hints (spec §06; see +`chart-builder.ts` `builderWarnings`): + +- **`distinct`** — the count of distinct non-empty values **in the sample**, + counted only up to `DISTINCT_CAP` (50). Past the cap the exact number stops + mattering — a legend or category axis with 50 entries is already unreadable — so + counting stops and **`distinctCapped`** flags that the real cardinality is at + least the cap. Powers the crowded-legend (discrete colour) and crowded-category + warnings. +- **`numericExtent`** — `{ min, max }` over the numeric values, for `number` + columns only (`null` otherwise), using the **same numeric test** as + `inferColumnType` so the two never disagree. Powers the negative-value **Size** + guard (size encodes magnitude → negatives mislead; Draco `hard.lp:56`). + +Both are derived in the same sample pass as type inference, so they are nearly +free, and both are **estimates** (sampled from the head, like the type) — fine for +soft advisories, not guarantees. URL / non-tabular data and datasets stored before +this field carry empty `columnStats`, and the dependent hints simply skip. + --- ## 4. Sampling vs. full scan `rowCount`/`columnCount`/`size` always reflect the **whole** dataset — they're -cheap (a length and a byte count). Only **type inference** has a per-value cost, -and it's the one place a huge dataset could hurt. +cheap (a length and a byte count). Only the **per-value** work — type inference +and the §3.3 column stats — has a per-row cost, and it's the one place a huge +dataset could hurt. -So: infer types from a **bounded sample** of rows, not the full column. A fixed -cap (e.g. the first ~200 rows) keeps profiling fast and predictable on large -pasted datasets while still being more than enough signal to classify a column. +So: derive types **and** stats from a **bounded sample** of rows, not the full +column. A fixed cap (e.g. the first ~200 rows) keeps profiling fast and +predictable on large pasted datasets while still being more than enough signal to +classify a column and estimate its cardinality / range. ```ts const SAMPLE_SIZE = 200; @@ -306,7 +346,8 @@ the same paste profiles the same way twice. ### Do / Don't - **Do** count rows/columns and size over the full payload. -- **Do** cap type-inference sampling at a fixed head slice for determinism. +- **Do** cap type-inference **and column-stats** sampling at a fixed head slice for + determinism (so `distinct` / `numericExtent` are estimates, like the type). - **Don't** randomly sample — non-deterministic profiles break tests and confuse users. - **Don't** scan every value of a million-row paste to guess a type. diff --git a/docs/chart-builder-research.md b/docs/chart-builder-research.md index f6a3813..9e04150 100644 --- a/docs/chart-builder-research.md +++ b/docs/chart-builder-research.md @@ -214,30 +214,29 @@ as of 2026-06-06. un-aggregated case. A general "long labels → go horizontal" suggestion on _any_ vertical bar is still deferred (needs a label-length / cardinality signal); a blanket warning was rejected — it would fire on every ordinary vertical bar. -- **A3 · Crowded-axis & high-cardinality warnings** _(partly done)_ — +- **A3 · Crowded-axis & high-cardinality warnings** _(done)_ — - _Done:_ the **un-aggregated crowded axis** — a bar/line/area with a category axis and a **raw** measure draws one mark (and one label) per row, so over `CROWDED_CATEGORY_ROWS` (30) rows it warns and points to aggregating, or a horizontal bar. Row-count-based: - `builderWarnings(config, rowCount)`, with `rowCount` from the loaded dataset; detects - exactly the mark-count == row-count case (URL/non-tabular → `rowCount` null → skipped). - - _Remaining (needs the profiling extension below):_ an **aggregated** axis that still has - many distinct **categories**, an unreadable **Color legend** (>10/>20 categories), and - **number-typed-Nominal**. All need per-column **cardinality**, which the profile lacks. -- **A4 · Data-aware Size guard** _(deferred — needs the profiling extension)_ — exclude - **negative**-valued columns from Size (Draco `hard.lp:56`; size implies positive - magnitude). Today only the type-level Size discipline is enforced. + detects exactly the mark-count == row-count case (URL/non-tabular → `rowCount` null → skipped). + - _Done (via the profiling extension below):_ an **aggregated** axis that still has many + distinct **categories** (`CROWDED_CATEGORY_DISTINCT` = 30) and an unreadable **Color + legend** (`CROWDED_LEGEND_DISTINCT` = 12, discrete colour only) now warn from per-column + cardinality. (Number-typed-Nominal remains a possible future nudge; not yet flagged.) +- **A4 · Data-aware Size guard** _(done)_ — Size mapped to a field whose profiled numeric + **extent** goes negative warns (Draco `hard.lp:56`; size implies positive magnitude), + on top of the type-level Size discipline. -> **TODO — profiling extension (the A3-remaining + A4 enabler).** `profile.ts` computes only -> `rowCount` / `columnCount` / `columnTypes`. Extend it, in the **same sample pass** that -> already feeds `inferColumnType` (so it's nearly free), to also derive per column: a -> **capped distinct count** (cardinality — cap at ~50; a sampled count is enough for a -> ">N categories" threshold, no full scan) and a **numeric extent** (min/max → sign). -> Surface them on `DatasetProfile` (alongside `columnTypes`). Then: `builderWarnings` consumes -> cardinality (legend/axis crowding) and the Size gate consumes sign (A4). **Caveats:** -> URL / non-tabular datasets have no rows at profile time → these fields are null and the -> dependent warnings simply skip; and stored datasets predate the field, so this needs a -> recompute-on-read or a `dataset-migrations` bump (see architecture 02 / 06). Keep -> thresholds in `chart-builder.ts` constants like `CROWDED_CATEGORY_ROWS`. +> **Done — profiling extension (the A3 / A4 enabler).** `profile.ts` now derives, in the +> **same sample pass** that feeds `inferColumnType`, a per-column **capped distinct count** +> (`DISTINCT_CAP` = 50, with a `distinctCapped` overflow flag) and a **numeric extent** +> (min/max, numeric columns only), surfaced on `DatasetProfile.columnStats` and the `Dataset` +> record. `builderWarnings(config, rowCount, columns)` consumes them for the legend/axis +> crowding hints and the negative-value Size guard. **Caveats handled:** URL / non-tabular +> data has no rows → `columnStats` is `[]` and the dependent hints skip; datasets stored +> before the field default to `[]` via `dataset-migrations` / import normalization (no forced +> re-profile on read — they pick up stats on next save). Thresholds live in `chart-builder.ts` +> constants alongside `CROWDED_CATEGORY_ROWS`. **B · Transform-enabled coverage (new core capability + §06 extension)** _(done)_ diff --git a/src/app/components/ChartBuilderModal.tsx b/src/app/components/ChartBuilderModal.tsx index b04a4f3..c7a6200 100644 --- a/src/app/components/ChartBuilderModal.tsx +++ b/src/app/components/ChartBuilderModal.tsx @@ -358,8 +358,14 @@ export function ChartBuilderModal() { // fresh array each render (which loops useSyncExternalStore — see SnippetStore note). const config = useChartBuilderStore((s) => s.config); const rowCount = useChartBuilderStore((s) => s.rowCount); + // `columns` is a stable reference set once at init (not rebuilt per render), so + // subscribing to it won't loop useSyncExternalStore. + const columns = useChartBuilderStore((s) => s.columns); const valid = useMemo(() => isBuilderConfigValid(config), [config]); - const warnings = useMemo(() => builderWarnings(config, rowCount), [config, rowCount]); + const warnings = useMemo( + () => builderWarnings(config, rowCount, columns), + [config, rowCount, columns], + ); const canSort = useMemo(() => supportsSort(config), [config]); const canStack = useMemo(() => supportsStack(config), [config]); diff --git a/src/app/infrastructure/dataset-migrations.ts b/src/app/infrastructure/dataset-migrations.ts index 718eec4..6c429a1 100644 --- a/src/app/infrastructure/dataset-migrations.ts +++ b/src/app/infrastructure/dataset-migrations.ts @@ -10,6 +10,7 @@ import { CURRENT_DATASET_VERSION, type DataSource, type Dataset } from '@core/dataset'; import type { DataFormat } from '@core/format-detection'; +import type { ColumnStats } from '@core/profile'; import type { ColumnType } from '@core/type-inference'; const FORMATS: ReadonlyArray = ['json', 'csv', 'tsv', 'topojson']; @@ -44,6 +45,10 @@ export function migrateDataset(raw: unknown): Dataset { columnTypes: Array.isArray(r.columnTypes) ? (r.columnTypes as Array<{ name: string; type: ColumnType }>) : [], + // Records predating cardinality/extent profiling default to empty stats; the + // Chart Builder's data-aware hints simply stay quiet until the dataset is + // re-saved (which re-profiles it) — graceful, no forced re-profile on read. + columnStats: Array.isArray(r.columnStats) ? (r.columnStats as ColumnStats[]) : [], size: typeof r.size === 'number' ? r.size : 0, created: typeof r.created === 'string' ? r.created : new Date(0).toISOString(), modified: typeof r.modified === 'string' ? r.modified : new Date(0).toISOString(), diff --git a/src/app/stores/ChartBuilderStore.ts b/src/app/stores/ChartBuilderStore.ts index 7a3ddaf..6b078c9 100644 Binary files a/src/app/stores/ChartBuilderStore.ts and b/src/app/stores/ChartBuilderStore.ts differ diff --git a/src/core/chart-builder.test.ts b/src/core/chart-builder.test.ts index 3f18f3b..34ed192 100644 --- a/src/core/chart-builder.test.ts +++ b/src/core/chart-builder.test.ts @@ -223,6 +223,206 @@ describe('builderWarnings (Tier B advisories)', () => { expect(hint?.message).toMatch(/reduce the number of categories/); }); }); + + describe('data-aware hints (from profiled column stats)', () => { + /** A BuilderColumns carrying stats for the named field. */ + const withStats = ( + name: string, + stats: { + distinct?: number; + distinctCapped?: boolean; + numericExtent?: { min: number; max: number } | null; + }, + ): BuilderColumns => ({ + columns: [name], + columnTypes: [{ name, type: 'string' }], + columnStats: [ + { + name, + distinct: stats.distinct ?? 1, + distinctCapped: stats.distinctCapped ?? false, + numericExtent: stats.numericExtent ?? null, + }, + ], + }); + + it('flags an aggregated category axis that still has too many distinct values', () => { + const w = builderWarnings( + { + datasetName: 'D', + mark: 'bar', + encodings: { + x: { field: 'sku', type: 'nominal' }, + y: { field: 'qty', type: 'quantitative', aggregate: 'sum' }, // aggregated, so not one-per-row + }, + }, + 500, + withStats('sku', { distinct: 48 }), + ); + 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 + }); + + it('reports "more than 50" when the category cardinality hit the profiler cap', () => { + const w = builderWarnings( + { + datasetName: 'D', + mark: 'bar', + encodings: { + x: { field: 'sku', type: 'nominal' }, + y: { field: 'qty', type: 'quantitative', aggregate: 'sum' }, + }, + }, + 500, + withStats('sku', { distinct: 50, distinctCapped: true }), + ); + expect(w.some((m) => /more than 50 distinct values/.test(m.message))).toBe(true); + }); + + it('is silent for a small-cardinality aggregated category axis', () => { + const w = builderWarnings( + { + datasetName: 'D', + mark: 'bar', + encodings: { + x: { field: 'sku', type: 'nominal' }, + y: { field: 'qty', type: 'quantitative', aggregate: 'sum' }, + }, + }, + 500, + withStats('sku', { distinct: 6 }), + ); + expect(w.some((m) => /distinct values/.test(m.message))).toBe(false); + }); + + it('warns when a discrete colour series has too many categories', () => { + const w = builderWarnings( + { + datasetName: 'D', + mark: 'bar', + encodings: { + x: { field: 'sku', type: 'nominal' }, + y: { field: 'qty', type: 'quantitative', aggregate: 'sum' }, + color: { field: 'tag', type: 'nominal' }, + }, + }, + 500, + { + columns: ['tag'], + columnTypes: [{ name: 'tag', type: 'string' }], + columnStats: [{ name: 'tag', distinct: 20, distinctCapped: false, numericExtent: null }], + }, + ); + const hint = w.find((m) => m.channel === 'color' && /legend/.test(m.message)); + expect(hint?.message).toMatch(/20 categories/); + }); + + it('does not flag a continuous (quantitative) colour ramp for cardinality', () => { + const w = builderWarnings( + { + datasetName: 'D', + mark: 'point', + encodings: { + x: { field: 'a', type: 'quantitative' }, + y: { field: 'b', type: 'quantitative' }, + color: { field: 'score', type: 'quantitative' }, // a ramp, no per-value legend + }, + }, + 500, + { + columns: ['score'], + columnTypes: [{ name: 'score', type: 'number' }], + columnStats: [ + { + name: 'score', + distinct: 50, + distinctCapped: true, + numericExtent: { min: 0, max: 9 }, + }, + ], + }, + ); + expect(w.some((m) => /legend/.test(m.message))).toBe(false); + }); + + it('guards Size against a field whose values go negative', () => { + const w = builderWarnings( + { + datasetName: 'D', + mark: 'point', + encodings: { + x: { field: 'a', type: 'quantitative' }, + y: { field: 'b', type: 'quantitative' }, + size: { field: 'delta', type: 'quantitative' }, + }, + }, + 500, + { + columns: ['delta'], + columnTypes: [{ name: 'delta', type: 'number' }], + columnStats: [ + { + name: 'delta', + distinct: 30, + distinctCapped: false, + numericExtent: { min: -12, max: 40 }, + }, + ], + }, + ); + const hint = w.find((m) => m.channel === 'size'); + expect(hint?.message).toMatch(/can't show negative values/); + expect(hint?.message).toMatch(/down to -12/); + }); + + it('does not guard Size when the field is wholly non-negative', () => { + const w = builderWarnings( + { + datasetName: 'D', + mark: 'point', + encodings: { + x: { field: 'a', type: 'quantitative' }, + y: { field: 'b', type: 'quantitative' }, + size: { field: 'amount', type: 'quantitative' }, + }, + }, + 500, + { + columns: ['amount'], + columnTypes: [{ name: 'amount', type: 'number' }], + columnStats: [ + { + name: 'amount', + distinct: 30, + distinctCapped: false, + numericExtent: { min: 0, max: 40 }, + }, + ], + }, + ); + expect(w.some((m) => m.channel === 'size')).toBe(false); + }); + + it('skips data-aware hints entirely when no column stats are supplied', () => { + // Old/URL datasets: same crowded config, but without stats the hints stay quiet. + const w = builderWarnings( + { + datasetName: 'D', + mark: 'bar', + encodings: { + x: { field: 'sku', type: 'nominal' }, + y: { field: 'qty', type: 'quantitative', aggregate: 'sum' }, + size: { field: 'qty', type: 'quantitative' }, + }, + }, + 500, + ); + expect(w.some((m) => /distinct values/.test(m.message))).toBe(false); + expect(w.some((m) => m.channel === 'size')).toBe(false); + }); + }); }); describe('defaultBuilderConfig', () => { diff --git a/src/core/chart-builder.ts b/src/core/chart-builder.ts index a9b7810..b2fa571 100644 --- a/src/core/chart-builder.ts +++ b/src/core/chart-builder.ts @@ -22,6 +22,8 @@ * editor and preview consume — `buildSnippetSpecText` serializes it. */ +import type { ColumnStats } from './profile'; +import { DISTINCT_CAP } from './profile'; import type { ColumnType } from './type-inference'; import { VEGA_LITE_SCHEMA_URL } from './snippet'; @@ -157,8 +159,9 @@ function isContinuous(type: FieldType): boolean { * implies ordered magnitude). Draco makes this a hard constraint * (`hard.lp:53` size_nominal); we surface it as a UI gate that disables Size for * unsuitable columns rather than letting the user produce the bad encoding. - * (Negative-value exclusion, `hard.lp:56`, needs row data and is left to the - * data-aware layer; this type-level gate is what the builder enforces.) + * (Negative-value exclusion, `hard.lp:56`, needs row data — `builderWarnings` + * surfaces it as a soft hint from the column's profiled numeric extent; this + * type-level gate is the part the builder can enforce structurally.) */ export function isChannelTypeAllowed(channel: ChannelName, type: FieldType): boolean { if (channel === 'size') return type === 'quantitative' || type === 'ordinal'; @@ -212,6 +215,13 @@ export function defaultMark(xType: FieldType | null, yType: FieldType | null): M export interface BuilderColumns { columns: readonly string[]; columnTypes: ReadonlyArray<{ name: string; type: ColumnType }>; + /** + * Per-column cardinality + numeric range, when the dataset was profiled with it + * (datasets created before this profiling, and URL/non-tabular data, omit it). + * The data-aware `builderWarnings` use it where present and silently skip those + * hints where absent. + */ + columnStats?: ReadonlyArray; } /** The derived field type for a named column, defaulting to Nominal if unknown. */ @@ -336,6 +346,31 @@ export interface BuilderWarning { */ const CROWDED_CATEGORY_ROWS = 30; +/** + * Above this many *distinct* category values, even an aggregated category axis (one + * mark per category, not per row) has too many labels to read. Matched to + * `CROWDED_CATEGORY_ROWS`: both flag a category axis that overruns its labels. + */ +const CROWDED_CATEGORY_DISTINCT = 30; + +/** + * Above this many distinct colour values a discrete legend is too long to scan and + * the palette starts recycling hues. Qualitative colour scales top out around 8–12 + * across the canon (Datawrapper, FT Visual Vocabulary); 12 is the generous end. + */ +const CROWDED_LEGEND_DISTINCT = 12; + +/** Profiled stats for a mapped field, when the dataset carries them. */ +function statsFor(field: string | undefined, columns?: BuilderColumns): ColumnStats | undefined { + if (!field || !columns?.columnStats) return undefined; + return columns.columnStats.find((s) => s.name === field); +} + +/** "N" or "more than 50" — distinct count, honouring the profiler's cap. */ +function cardinalityText(stats: ColumnStats): string { + return stats.distinctCapped ? `more than ${DISTINCT_CAP}` : `${stats.distinct}`; +} + /** * 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 @@ -343,11 +378,17 @@ const CROWDED_CATEGORY_ROWS = 30; * ConfigValid` is the only gate — they just steer the user toward a better chart. * Returned in a stable order so the UI list doesn't jitter as config changes. * - * `rowCount` (the dataset's row count, when known) powers the crowded-axis hint; - * pass it from the loaded dataset. Omitted/`null` (URL or non-tabular data) simply - * skips that one hint. + * `rowCount` (the dataset's row count, when known) powers the one-mark-per-row hint; + * `columns` (with per-column `columnStats`) powers the data-aware hints — crowded + * legend / category axis (cardinality) and the negative-value Size guard (numeric + * extent). Either omitted (URL / non-tabular / pre-cardinality datasets) simply + * skips the hints that need it. */ -export function builderWarnings(config: BuilderConfig, rowCount?: number | null): BuilderWarning[] { +export function builderWarnings( + config: BuilderConfig, + rowCount?: number | null, + columns?: BuilderColumns, +): BuilderWarning[] { const warnings: BuilderWarning[] = []; const x = config.encodings.x ?? null; const y = config.encodings.y ?? null; @@ -400,32 +441,74 @@ export function builderWarnings(config: BuilderConfig, rowCount?: number | null) }); } - // Crowded category axis: a bar/line/area pairing a discrete category against a - // *raw* (un-aggregated, un-binned) measure draws one mark — and one axis label — - // per row, so a large dataset becomes an unreadable picket fence of labels (the - // builder's own default does this: first column on X, second on Y, no aggregate). - // We can only flag the un-aggregated case, where mark-count == rowCount exactly; - // an *aggregated* axis that still has many distinct categories needs per-column - // distinct counts we don't profile yet (backlog A2/A3). The fix follows the canon: - // aggregate the measure to one mark per category, or — for a bar — flip to a - // horizontal bar where long labels stay readable (FT Visual Vocabulary: bar is - // "good when … labels have long category names"; Datawrapper: long category lists - // belong on a horizontal bar). - if ( - (mark === 'bar' || mark === 'line' || mark === 'area') && - typeof rowCount === 'number' && - rowCount > CROWDED_CATEGORY_ROWS - ) { - const category = sortableCategoryChannel(config); // the discrete axis of a category-vs-measure pair + // Crowded category axis: a bar/line/area whose discrete category axis carries too + // many entries to label legibly. Two ways it happens, mutually exclusive: + // 1. A *raw* (un-aggregated, un-binned) measure draws one mark per row, so the + // label count == rowCount — flagged from rowCount alone (the builder's own + // default does this: first column on X, second on Y, no aggregate). + // 2. Otherwise (aggregated/binned), one mark per *category* — crowded only when + // the category column itself has many distinct values, which needs the + // profiled cardinality (columnStats) and is skipped without it. + // The remedy follows the canon: for case 1 aggregate to one mark per category; for + // either, a bar can flip to horizontal where long labels stay readable (FT Visual + // Vocabulary: bar is "good when … labels have long category names"; Datawrapper: + // 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 && !measure.aggregate && !measure.bin) { - 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.'; + if (category && 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.'; + warnings.push({ + channel: category, + message: `This draws one mark per row (${rowCount} in this dataset), so the category-axis labels will overlap. ${fix}`, + }); + } 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".'; + warnings.push({ + channel: category, + message: `This category axis has ${cardinalityText(stats)} distinct values, so its labels will overlap. ${fix}`, + }); + } + } + } + } + + // Crowded colour legend: a discrete colour series with many distinct values makes + // the legend too long to scan and forces the palette to recycle hues. A continuous + // colour ramp (quantitative/temporal) has no per-value legend, so this is for + // discrete colour only (Datawrapper/FT cap categorical colour low). + const color = config.encodings.color ?? null; + if (color && !isMeasureMapping(color)) { + const stats = statsFor(color.field, columns); + if (stats && stats.distinct > CROWDED_LEGEND_DISTINCT) { warnings.push({ - channel: category, - message: `This draws one mark per row (${rowCount} in this dataset), so the category-axis labels will overlap. ${fix}`, + channel: 'color', + message: `Colour has ${cardinalityText(stats)} categories — a legend that long is hard to scan and the palette will repeat hues. Group rarer categories, or map this field to the X axis instead.`, + }); + } + } + + // Negative-value Size guard (Draco hard.lp:56 size_negative): symbol size encodes + // magnitude, so negatives render as zero/clipped area and mislead. The type gate + // (isChannelTypeAllowed) can't catch this — it needs the data — so we flag it from + // the profiled numeric extent. + const size = config.encodings.size ?? null; + if (size?.field) { + const stats = statsFor(size.field, columns); + if (stats?.numericExtent && stats.numericExtent.min < 0) { + warnings.push({ + channel: 'size', + message: `Size can't show negative values, but "${size.field}" goes down to ${stats.numericExtent.min}. Negative magnitudes render as tiny or clipped symbols — encode this field with Colour (a diverging scale) instead, or filter to non-negative values.`, }); } } diff --git a/src/core/dataset.ts b/src/core/dataset.ts index 995b1c9..1844f6d 100644 --- a/src/core/dataset.ts +++ b/src/core/dataset.ts @@ -18,7 +18,7 @@ */ import type { DataFormat } from './format-detection'; -import { profileData, type DatasetProfile } from './profile'; +import { profileData, type ColumnStats, type DatasetProfile } from './profile'; import type { ColumnType } from './type-inference'; /** Current schema version for a Dataset record (read-time migration target). */ @@ -53,6 +53,8 @@ export interface Dataset { columns: string[]; /** Per-column inferred type. */ columnTypes: Array<{ name: string; type: ColumnType }>; + /** Per-column cardinality + numeric range (empty for URL / non-tabular). */ + columnStats: ColumnStats[]; /** Approximate payload size in bytes. */ size: number; /** ISO timestamp — when first added. */ @@ -243,9 +245,10 @@ export interface CreateDatasetOptions { /** * Create a Dataset: stamps version/timestamps and runs `computeDatasetProfile` to - * fill the derived summary fields. `id` defaults to `Date.now()` (numeric; - * collision-prone for batch creation — see the DatasetStore.add TODO) and is - * injectable for tests. + * fill the derived summary fields. `id` defaults to `Date.now()` (numeric) and is + * injectable for tests; note that `DatasetStore` is the id authority and reassigns + * a collision-free id on insertion (`nextDatasetId`), so this default never reaches + * storage through the normal create paths. */ export function createDataset(options: CreateDatasetOptions): Dataset { const now = options.now ?? new Date(); @@ -264,6 +267,7 @@ export function createDataset(options: CreateDatasetOptions): Dataset { columnCount: profile.columnCount, columns: profile.columns, columnTypes: profile.columnTypes, + columnStats: profile.columnStats, size: profile.size, created: iso, modified: iso, diff --git a/src/core/import-normalize.test.ts b/src/core/import-normalize.test.ts index aa02f88..7c1b24b 100644 --- a/src/core/import-normalize.test.ts +++ b/src/core/import-normalize.test.ts @@ -59,6 +59,10 @@ function datasetRecord(over: Partial = {}): Dataset { { name: 'a', type: 'number' }, { name: 'b', type: 'number' }, ], + columnStats: [ + { name: 'a', distinct: 1, distinctCapped: false, numericExtent: { min: 1, max: 1 } }, + { name: 'b', distinct: 1, distinctCapped: false, numericExtent: { min: 2, max: 2 } }, + ], size: 7, created: '2025-01-01T00:00:00.000Z', modified: '2025-01-01T00:00:00.000Z', diff --git a/src/core/import-normalize.ts b/src/core/import-normalize.ts index eab17e5..0e35b2d 100644 --- a/src/core/import-normalize.ts +++ b/src/core/import-normalize.ts @@ -16,6 +16,7 @@ import { CURRENT_DATASET_VERSION, type DataSource, type Dataset } from './dataset'; import type { DataFormat } from './format-detection'; import { makeUniqueName } from './naming'; +import type { ColumnStats } from './profile'; import { CURRENT_SNIPPET_VERSION, type Snippet } from './snippet'; import { extractDatasetRefs, recomputeDatasetRefs, renameDatasetInSpec } from './spec-refs'; import type { ColumnType } from './type-inference'; @@ -173,6 +174,7 @@ function normalizeDataset(raw: unknown, nowIso: string): Dataset { columnTypes: Array.isArray(r.columnTypes) ? (r.columnTypes as Array<{ name: string; type: ColumnType }>) : [], + columnStats: Array.isArray(r.columnStats) ? (r.columnStats as ColumnStats[]) : [], size: typeof r.size === 'number' ? r.size : 0, created, modified, diff --git a/src/core/profile.test.ts b/src/core/profile.test.ts index b7b6a44..555f203 100644 --- a/src/core/profile.test.ts +++ b/src/core/profile.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vitest'; -import { profileData } from './profile'; +import { DISTINCT_CAP, profileData } from './profile'; describe('profileData', () => { test('profiles a JSON-array dataset fully', () => { @@ -53,3 +53,47 @@ describe('profileData', () => { expect(profile.columnTypes).toEqual([{ name: 'v', type: 'number' }]); }); }); + +describe('profileData — column stats (cardinality + numeric extent)', () => { + test('counts distinct values and reports numeric extent for number columns', () => { + const rows = [ + { region: 'N', sales: 10 }, + { region: 'S', sales: -4 }, + { region: 'N', sales: 25 }, // region repeats → 2 distinct, not 3 + ]; + const stats = profileData(rows, 0).columnStats; + expect(stats).toEqual([ + { name: 'region', distinct: 2, distinctCapped: false, numericExtent: null }, + { name: 'sales', distinct: 3, distinctCapped: false, numericExtent: { min: -4, max: 25 } }, + ]); + }); + + test('distinct counting is empty-insensitive and case/whitespace literal', () => { + const rows = [{ c: 'A' }, { c: ' A ' }, { c: '' }, { c: null }, { c: 'b' }]; + // ' A ' trims to 'A' (one key); '' and null are empties (no signal) → 2 distinct. + expect(profileData(rows, 0).columnStats[0]).toEqual({ + name: 'c', + distinct: 2, + distinctCapped: false, + numericExtent: null, + }); + }); + + test('caps distinct at DISTINCT_CAP and flags the overflow', () => { + const rows = Array.from({ length: DISTINCT_CAP + 25 }, (_, i) => ({ id: `v${i}` })); + const stat = profileData(rows, 0).columnStats[0]; + expect(stat.distinct).toBe(DISTINCT_CAP); + expect(stat.distinctCapped).toBe(true); + }); + + test('numericExtent is null for a non-numeric column even if some cells parse', () => { + // A 'string' column (one non-numeric value knocks the type down) carries no extent. + const rows = [{ v: '1' }, { v: '2' }, { v: 'oops' }]; + const stat = profileData(rows, 0).columnStats[0]; + expect(stat.numericExtent).toBeNull(); + }); + + test('N/A profile carries empty column stats', () => { + expect(profileData(null, 9).columnStats).toEqual([]); + }); +}); diff --git a/src/core/profile.ts b/src/core/profile.ts index f6ac29d..dd3c83b 100644 --- a/src/core/profile.ts +++ b/src/core/profile.ts @@ -19,7 +19,7 @@ * determinism (arch 06 §4). */ -import { inferColumnType, type ColumnType } from './type-inference'; +import { inferColumnType, isEmpty, isNumeric, type ColumnType } from './type-inference'; export interface ColumnTypeInfo { /** The column name. */ @@ -28,6 +28,33 @@ export interface ColumnTypeInfo { type: ColumnType; } +/** + * Cap on distinct values counted per column. Past this the exact count stops + * mattering for the chart-guidance hints (a legend or category axis with 50 + * entries is already unreadable), so counting stops and `distinctCapped` flags + * that the true cardinality is at least this. Sampled like type inference (head of + * the data), so it is an estimate — fine for soft advisories, not a guarantee. + */ +export const DISTINCT_CAP = 50; + +/** + * Per-column cardinality + numeric range, derived in the same sample pass as type + * inference (arch 06 §4) to power the Chart Builder's data-aware Tier-B hints: + * a crowded colour legend / category axis (`distinct`), and the negative-value + * Size guard (`numericExtent`). Distinct from `ColumnTypeInfo` because it answers + * "what shape is this data" rather than "what type to encode it as". + */ +export interface ColumnStats { + /** Column name (matches a `columns` entry). */ + name: string; + /** Distinct non-empty values in the sample, capped at `DISTINCT_CAP`. */ + distinct: number; + /** True when `distinct` hit the cap — the real cardinality is ≥ `DISTINCT_CAP`. */ + distinctCapped: boolean; + /** Min/max over numeric values, for numeric columns only; `null` otherwise. */ + numericExtent: { min: number; max: number } | null; +} + export interface DatasetProfile { /** Data rows, or `null` when N/A (URL / non-tabular). */ rowCount: number | null; @@ -37,6 +64,8 @@ export interface DatasetProfile { columns: string[]; /** Per-column inferred type. */ columnTypes: ColumnTypeInfo[]; + /** Per-column cardinality + numeric range (empty when N/A). */ + columnStats: ColumnStats[]; /** Approximate payload size in bytes. */ size: number; } @@ -47,6 +76,7 @@ const naProfile = (size: number): DatasetProfile => ({ columnCount: null, columns: [], columnTypes: [], + columnStats: [], size, }); @@ -59,6 +89,45 @@ const SAMPLE_SIZE = 200; const sampleRows = (rows: ReadonlyArray): ReadonlyArray => rows.length <= SAMPLE_SIZE ? rows : rows.slice(0, SAMPLE_SIZE); +/** A canonical key for distinct-counting (trim strings; stringify the rest). */ +const distinctKey = (v: unknown): string => (typeof v === 'string' ? v.trim() : String(v)); + +/** + * Cardinality + numeric extent for one column's sampled values. Distinct counting + * stops growing the set once it passes `DISTINCT_CAP` (the extra value reveals the + * overflow), but the extent scan continues over every value. Extent is reported + * only for `number` columns — the only ones the Size channel can encode — using + * the same numeric test as type inference, so the two never disagree. + */ +function columnStatsFor(name: string, values: readonly unknown[], type: ColumnType): ColumnStats { + const seen = new Set(); + let capped = false; + let min = Infinity; + let max = -Infinity; + let sawNumber = false; + + for (const v of values) { + if (isEmpty(v)) continue; + if (!capped) { + seen.add(distinctKey(v)); + if (seen.size > DISTINCT_CAP) capped = true; + } + if (type === 'number' && isNumeric(v)) { + sawNumber = true; + const n = typeof v === 'number' ? v : Number(String(v).trim()); + if (n < min) min = n; + if (n > max) max = n; + } + } + + return { + name, + distinct: capped ? DISTINCT_CAP : seen.size, + distinctCapped: capped, + numericExtent: sawNumber ? { min, max } : null, + }; +} + /** * Profile a dataset payload. `rows` is the tabular form (CSV/TSV/JSON-array) * already parsed to rows-of-objects, or `null` for URL / non-tabular data; @@ -84,20 +153,25 @@ export function profileData( if (columns.length === 0) return naProfile(size); const sample = sampleRows(rows); - // TODO (backlog: docs/chart-builder-research.md §8 — A3/A4 enabler): in this same - // sample pass, also derive a capped per-column distinct count (cardinality, cap ~50) - // and numeric extent (min/max → sign), surfaced on DatasetProfile, to power the Chart - // Builder's crowded-legend / high-cardinality warnings and its negative-value Size guard. - const columnTypes = columns.map((name) => ({ - name, - type: inferColumnType(sample.map((r) => r[name])), - })); + // One pass per column over the sample: the inferred display type, plus the + // cardinality + numeric extent that power the Chart Builder's data-aware hints + // (crowded legend / category axis, negative-value Size guard — see chart-builder + // `builderWarnings`; docs/chart-builder-research.md §8). + const columnTypes: ColumnTypeInfo[] = []; + const columnStats: ColumnStats[] = []; + for (const name of columns) { + const values = sample.map((r) => r[name]); + const type = inferColumnType(values); + columnTypes.push({ name, type }); + columnStats.push(columnStatsFor(name, values, type)); + } return { rowCount: rows.length, columnCount: columns.length, columns, columnTypes, + columnStats, size, }; } diff --git a/src/core/type-inference.ts b/src/core/type-inference.ts index 2651dc9..900979d 100644 --- a/src/core/type-inference.ts +++ b/src/core/type-inference.ts @@ -24,11 +24,11 @@ export type ColumnType = 'number' | 'string' | 'date' | 'boolean'; /** Empty cells (null/undefined/whitespace-only string) carry no type signal. */ -const isEmpty = (v: unknown): boolean => +export const isEmpty = (v: unknown): boolean => v === null || v === undefined || (typeof v === 'string' && v.trim() === ''); /** Native numbers pass when finite; strings must parse to a finite, non-NaN number. */ -const isNumeric = (v: unknown): boolean => { +export const isNumeric = (v: unknown): boolean => { if (typeof v === 'number') return Number.isFinite(v); if (typeof v !== 'string') return false; const t = v.trim();