Skip to content

Commit ac89727

Browse files
committed
feat(engine)!: reject point-mark sizes above 50px radius
openchart point size is a radius in px (default 5), but Vega-Lite — the prior every LLM and most authors carry — defines point size as an area in px². VL-habit values (60-900) therefore render as chart-swallowing discs, and this mistake kept recurring in downstream chart authoring. validateSpec now errors on mark.size, encoding.size.value, and encoding.size.scale.range values above 50 for point marks, with the sqrt(area/pi) conversion recipe in the message. Docstrings on MarkDef.size and the size channel now document radius semantics honestly (the size channel previously claimed VL alignment).
1 parent 3e56038 commit ac89727

5 files changed

Lines changed: 185 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this
88

99
### Breaking Changes
1010

11+
- **validate!:** point-mark sizes above 50 now fail validation. Openchart point sizes are radii in px (`mark.size` default 5; `encoding.size` range default [3, 30]), but Vega-Lite defines point `size` as an area in px², so VL-habit values (60-900) rendered as chart-swallowing discs and were, in practice, always a units mistake. The error carries the conversion (r = sqrt(area/pi), e.g. VL 110 → `size: 6`) on `mark.size`, `encoding.size.value`, and `encoding.size.scale.range`. Intentional radii up to 50 stay valid; `MarkDef.size` and the `size` channel docs now state the units per mark type.
12+
1113
- **bar/area!:** multi-series bar and area charts now default to stacked (`stack: 'zero'`) when a `color` encoding is present, matching Vega-Lite. Pass `stack: null` on the value channel to keep the old grouped (bar) or overlap (area) behavior. See [migration guide](docs/migrating-v8.md#1-multi-series-bararea-charts-default-to-stacked).
1214
- **arc!:** `encoding.theta` is now the canonical value channel for arc, waffle, and parliament marks. `encoding.y` still works at runtime (sugar rewrites it to theta with a deprecation warning) but is removed from the TypeScript type.
1315
- **encoding!:** dead channels `shape`, `radius`, `href`, and `order` removed from the `Encoding` interface and `MARK_ENCODING_RULES`. They were never implemented. Runtime strips them with a warning for backward compat.

docs/spec-reference.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ The engine validates encoding channels at runtime using `CHART_ENCODING_RULES`.
315315

316316
**color channel**: Series differentiation. Assigns colors from the categorical palette. Required for pie/donut (defines slices). Optional for all others (creates multi-series with legend).
317317

318-
**size channel**: Mark size. Creates bubble charts when applied to scatter. Maps a quantitative field to the mark radius or area.
318+
**size channel**: Mark size. Creates bubble charts when applied to scatter. Maps a quantitative field to the dot **radius in pixels**, area-proportionally (sqrt curve); the default range is [3, 30] and `scale.range` overrides it in radius px. A constant lives at `mark.size`, also a radius (default 5, typical 2-12). Note this differs from Vega-Lite, where point `size` is an **area** in px² — a VL size of 110 is `mark.size: 6` here (r = √(area/π)); validation rejects radii above 50 with the conversion.
319319

320320
**detail channel**: Grouping without visual encoding. Splits data into groups (like color does) but doesn't assign different colors. Useful when you want separate lines per group but all the same color.
321321

packages/core/src/types/spec.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,15 @@ export interface MarkDef {
219219
endAngle?: number;
220220
/** Corner radius for rect/bar marks. 'pill' sets rx to half the bar thickness. */
221221
cornerRadius?: number | 'pill';
222-
/** Fixed bar thickness in pixels for bar/column marks. When set, bars are this height (horizontal) or width (vertical), centered within the band. */
222+
/**
223+
* Bar/column marks: fixed bar thickness in pixels. When set, bars are this
224+
* height (horizontal) or width (vertical), centered within the band.
225+
*
226+
* Point marks: the dot RADIUS in pixels (default 5, typical 2-12). This is
227+
* NOT Vega-Lite's point `size`, which is an area in px^2 -- a VL size of
228+
* 110 corresponds to `size: 6` here (r = sqrt(area / pi)). Validation
229+
* rejects values above 50 with the conversion.
230+
*/
223231
size?: number;
224232
/** Whether the mark is filled (vs stroked only). */
225233
filled?: boolean;
@@ -701,9 +709,13 @@ export interface Encoding<TData extends DataRow = DataRow> {
701709
*/
702710
color?: EncodingChannel<TData> | ConditionalValueDef<TData> | ValueDef;
703711
/**
704-
* Size channel. Used by point/bubble charts to scale dot area by a quantitative field.
705-
* Accepts a conditional definition to vary size based on data predicates,
706-
* or a bare `{ value }` constant (VL aligned; expanded to `mark.size`).
712+
* Size channel. Used by point/bubble charts to scale dots by a quantitative
713+
* field, area-proportionally (sqrt curve). The units are dot RADII in px:
714+
* `scale.range` defaults to [3, 30] for scatter, and a bare `{ value }`
715+
* constant expands to `mark.size`, also a radius. This differs from
716+
* Vega-Lite, where point size is an area in px^2 -- VL-scale values
717+
* (60-900) are rejected by validation with the conversion.
718+
* Accepts a conditional definition to vary size based on data predicates.
707719
*/
708720
size?: EncodingChannel<TData> | ConditionalValueDef<TData> | ValueDef;
709721
/**
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/**
2+
* Tests for the Vega-Lite point-size near-miss guard.
3+
*
4+
* VL defines point `size` as an area in px^2; openchart's `mark.size` and
5+
* `encoding.size` ranges are radii in px. Values above the plausibility
6+
* ceiling are always imported VL area numbers in practice, so they must fail
7+
* validation with the radius conversion in the suggestion.
8+
*/
9+
10+
import { describe, expect, it } from 'vitest';
11+
import { validateSpec } from '../validate';
12+
13+
const DATA = [
14+
{ spend: 10_266, score: 219.3 },
15+
{ spend: 29_720, score: 214.6 },
16+
];
17+
18+
function scatter(overrides: Record<string, unknown> = {}) {
19+
return {
20+
mark: { type: 'point', filled: true },
21+
data: DATA,
22+
encoding: {
23+
x: { field: 'spend', type: 'quantitative' },
24+
y: { field: 'score', type: 'quantitative' },
25+
},
26+
...overrides,
27+
};
28+
}
29+
30+
describe('point size validation', () => {
31+
it('accepts radius-scale mark.size values', () => {
32+
const result = validateSpec(scatter({ mark: { type: 'point', size: 6 } }));
33+
expect(result.valid).toBe(true);
34+
});
35+
36+
it('accepts the ceiling itself', () => {
37+
const result = validateSpec(scatter({ mark: { type: 'point', size: 50 } }));
38+
expect(result.valid).toBe(true);
39+
});
40+
41+
it('rejects VL-area mark.size values with the radius conversion', () => {
42+
const result = validateSpec(scatter({ mark: { type: 'point', size: 110 } }));
43+
expect(result.valid).toBe(false);
44+
const error = result.errors.find((e) => e.path === 'mark.size');
45+
expect(error).toBeDefined();
46+
expect(error?.code).toBe('INVALID_VALUE');
47+
// sqrt(110 / pi) ~ 5.9 -> 6
48+
expect(error?.suggestion).toContain('use 6');
49+
expect(error?.suggestion).toContain('radi');
50+
});
51+
52+
it('rejects a VL-area encoding.size.value constant', () => {
53+
const spec = scatter();
54+
(spec.encoding as Record<string, unknown>).size = { value: 130 };
55+
const result = validateSpec(spec);
56+
expect(result.valid).toBe(false);
57+
expect(result.errors.some((e) => e.path === 'encoding.size.value')).toBe(true);
58+
});
59+
60+
it('rejects VL-area encoding.size scale ranges', () => {
61+
const spec = scatter();
62+
(spec.encoding as Record<string, unknown>).size = {
63+
field: 'spend',
64+
type: 'quantitative',
65+
scale: { range: [80, 900] },
66+
};
67+
const result = validateSpec(spec);
68+
expect(result.valid).toBe(false);
69+
const error = result.errors.find((e) => e.path === 'encoding.size.scale.range');
70+
expect(error).toBeDefined();
71+
});
72+
73+
it('accepts radius-scale encoding.size ranges', () => {
74+
const spec = scatter();
75+
(spec.encoding as Record<string, unknown>).size = {
76+
field: 'spend',
77+
type: 'quantitative',
78+
scale: { range: [4, 18] },
79+
};
80+
expect(validateSpec(spec).valid).toBe(true);
81+
});
82+
83+
it('leaves bar mark.size (thickness px) alone', () => {
84+
const result = validateSpec({
85+
mark: { type: 'bar', size: 60 },
86+
data: [
87+
{ state: 'UT', score: 219.3 },
88+
{ state: 'NY', score: 214.6 },
89+
],
90+
encoding: {
91+
x: { field: 'state', type: 'nominal' },
92+
y: { field: 'score', type: 'quantitative' },
93+
},
94+
});
95+
expect(result.valid).toBe(true);
96+
});
97+
});

packages/engine/src/compiler/validate.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,71 @@ function validateMarkRenderMode(spec: Record<string, unknown>, errors: Validatio
455455
}
456456
}
457457

458+
// ---------------------------------------------------------------------------
459+
// Point size validation
460+
// ---------------------------------------------------------------------------
461+
462+
/**
463+
* Ceiling above which a point size is, in practice, always an imported
464+
* Vega-Lite *area* value rather than an intentional radius. VL defines point
465+
* `size` as symbol area in px^2 (size: 110 is a ~12px-wide dot); openchart's
466+
* `mark.size` and `encoding.size` ranges are a radius in px (default 5), so
467+
* VL-habit values render as chart-swallowing discs. Radii up to the ceiling
468+
* stay legal: a 50px bubble is enormous but plausible as a deliberate choice.
469+
*/
470+
const MAX_PLAUSIBLE_POINT_RADIUS = 50;
471+
472+
function pointSizeError(path: string, value: number): ValidationError {
473+
const radius = Math.max(1, Math.round(Math.sqrt(value / Math.PI)));
474+
return {
475+
message: `Spec error: ${path} is a radius in pixels, and ${value} would draw a ${Math.round(value * 2)}px-wide circle. This usually means the value was written in Vega-Lite's units, where point size is an area in px^2.`,
476+
path,
477+
code: 'INVALID_VALUE',
478+
suggestion: `Point sizes are radii in px (default 5, typical 2-12). For the dot a Vega-Lite size of ${value} draws, use ${radius} (r = sqrt(area / pi)).`,
479+
};
480+
}
481+
482+
/**
483+
* Guard against Vega-Lite size semantics on point marks -- the same
484+
* near-miss-VL treatment as `data.url` and string `calculate` expressions:
485+
* a form that would silently do the wrong thing fails loud with the recipe.
486+
*/
487+
function validatePointSize(spec: Record<string, unknown>, errors: ValidationError[]): void {
488+
const markType =
489+
typeof spec.mark === 'string' ? spec.mark : (spec.mark as Record<string, unknown>)?.type;
490+
if (markType !== 'point') return;
491+
492+
if (spec.mark && typeof spec.mark === 'object') {
493+
const size = (spec.mark as Record<string, unknown>).size;
494+
if (typeof size === 'number' && size > MAX_PLAUSIBLE_POINT_RADIUS) {
495+
errors.push(pointSizeError('mark.size', size));
496+
}
497+
}
498+
499+
const sizeEnc =
500+
spec.encoding && typeof spec.encoding === 'object'
501+
? ((spec.encoding as Record<string, unknown>).size as Record<string, unknown> | undefined)
502+
: undefined;
503+
if (!sizeEnc || typeof sizeEnc !== 'object') return;
504+
505+
if (typeof sizeEnc.value === 'number' && sizeEnc.value > MAX_PLAUSIBLE_POINT_RADIUS) {
506+
errors.push(pointSizeError('encoding.size.value', sizeEnc.value));
507+
}
508+
509+
const range =
510+
sizeEnc.scale && typeof sizeEnc.scale === 'object'
511+
? (sizeEnc.scale as Record<string, unknown>).range
512+
: undefined;
513+
if (Array.isArray(range)) {
514+
const worst = range.find(
515+
(entry) => typeof entry === 'number' && entry > MAX_PLAUSIBLE_POINT_RADIUS,
516+
);
517+
if (typeof worst === 'number') {
518+
errors.push(pointSizeError('encoding.size.scale.range', worst));
519+
}
520+
}
521+
}
522+
458523
// ---------------------------------------------------------------------------
459524
// Chart validation
460525
// ---------------------------------------------------------------------------
@@ -613,6 +678,10 @@ function validateChartSpec(spec: Record<string, unknown>, errors: ValidationErro
613678
// but a value outside the union is a typo and fails loud.
614679
validateMarkRenderMode(spec, errors);
615680

681+
// Point sizes that only make sense as Vega-Lite areas fail loud with the
682+
// radius conversion, before they render as chart-swallowing discs.
683+
validatePointSize(spec, errors);
684+
616685
// Near-miss: VL's string expression form of calculate. A restricted string
617686
// grammar is deliberately not supported (decision: structured form only);
618687
// point authors at the structured equivalent instead.

0 commit comments

Comments
 (0)