From b3b6f12968ac9ac03ec90a89731c46bf5f6f22e1 Mon Sep 17 00:00:00 2001 From: taoche Date: Fri, 4 Sep 2026 15:58:18 +0800 Subject: [PATCH 1/4] feat(core): export chart validation from flint-chart (#104) Move the validate/assemble logic that lived only inside flint-chart-mcp into the core package as `flint-chart/validate` (also re-exported from the root). Hosts that let an agent author ChartAssemblyInput outside MCP can now call validateChart(input, backend) and get the same per-problem feedback the validate_chart tool provides, without vendoring MCP sources. - validateChart / validateChartInput / assembleForBackend / stripPrivateKeys - validateSemanticTypes: unregistered semantic_types labels, surfaced by validateChart as unknown_semantic_type warnings - isRegistered / getRegisteredTypes exported from flint-chart/core - flint-chart-mcp now consumes the core implementation; it only adds data.url resolution on top Co-Authored-By: Claude Code --- CHANGELOG.md | 13 + docs/api-reference.md | 31 ++ docs/zh-CN/api-reference.md | 29 ++ packages/flint-js/README.md | 1 + packages/flint-js/package.json | 5 + packages/flint-js/src/core/index.ts | 1 + packages/flint-js/src/index.ts | 8 + packages/flint-js/src/validate/index.ts | 341 ++++++++++++++++++++++ packages/flint-js/tests/validate.test.ts | 168 +++++++++++ packages/flint-js/tsup.config.ts | 1 + packages/flint-mcp/src/render/assemble.ts | 194 ++---------- packages/flint-mcp/src/tools/validate.ts | 50 ++-- 12 files changed, 643 insertions(+), 199 deletions(-) create mode 100644 packages/flint-js/src/validate/index.ts create mode 100644 packages/flint-js/tests/validate.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 40c7ecc1..a11cf91b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Chart validation is now part of the core package. `validateChart(input, + backend)` returns `{ valid, warnings, errors, computedSize }` without + throwing, alongside `validateChartInput`, `validateSemanticTypes`, + `assembleForBackend`, and `stripPrivateKeys`, from `flint-chart` and the new + `flint-chart/validate` subpath. Hosts that let an agent author chart inputs + outside MCP get the same per-problem feedback the `validate_chart` tool + provides; `flint-chart-mcp` now consumes this implementation. Unregistered + `semantic_types` labels surface as `unknown_semantic_type` warnings, and + `isRegistered` / `getRegisteredTypes` are exported from `flint-chart/core` + ([#104](https://github.com/microsoft/flint-chart/issues/104)). + ### Fixed - Keyboard targeting now navigates and emits `focus-element` through the diff --git a/docs/api-reference.md b/docs/api-reference.md index 5c67e97a..52aac166 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -292,6 +292,36 @@ Re-exported from `flint-chart` and `flint-chart/core`: Key types: `ChartAssemblyInput`, `ChartEncoding`, `ChartTemplateDef`, `AssembleOptions`, `ChartWarning`, `ChannelSemantics`. +## Validation + +Hosts that let an agent author a `ChartAssemblyInput` can validate it before +rendering. `validateChart` never throws; it reports every warning the assembler +emits plus a single `assembly_failed` error when the input cannot be compiled +(unknown chart type, unsupported channel, nonexistent field, canvas caps). +Re-exported from `flint-chart` and `flint-chart/validate`: + +```ts +import { validateChart } from 'flint-chart/validate'; + +const result = validateChart(input, 'vegalite'); +// { backend, chartType, valid, warnings, errors, computedSize? } +if (!result.valid) { + // feed result.errors back to the agent +} +``` + +| Symbol | Purpose | +|--------|---------| +| `validateChart(input, backend, options?)` | Validate and assemble; never throws | +| `validateChartInput(input, backend?, options?)` | Shape checks only; throws on the first problem | +| `validateSemanticTypes(semantic_types)` | Labels not in the type registry (reported by `validateChart` as `unknown_semantic_type` warnings) | +| `assembleForBackend(backend, input, options?)` | Assemble and split out `_warnings` / `_width` / `_height` | +| `stripPrivateKeys(spec)` | Remove Flint's `_`-prefixed metadata from a spec | + +`options.maxDataRows` (default 100,000) and `options.maxCanvasDim` (default +4000) cap input size. Inline `data.values` are required — resolve `data.url` +to rows before validating. + --- # §8 Overflow and warnings @@ -325,6 +355,7 @@ Inspect `_warnings` or `ChartWarning` arrays in integration code to surface trun | `flint-chart/vegalite` | VL templates and `assembleVegaLite` | | `flint-chart/echarts` | ECharts templates and `assembleECharts` | | `flint-chart/chartjs` | Chart.js templates and `assembleChartjs` | +| `flint-chart/validate` | `validateChart` and input validation helpers | | `flint-chart/test-data` | Gallery generators (`TEST_GENERATORS`) | --- diff --git a/docs/zh-CN/api-reference.md b/docs/zh-CN/api-reference.md index ad2fb88d..3eb4e28f 100644 --- a/docs/zh-CN/api-reference.md +++ b/docs/zh-CN/api-reference.md @@ -255,6 +255,34 @@ vlGetTemplateChannels('Scatter Plot'); 关键类型：`ChartAssemblyInput`、`ChartEncoding`、`ChartTemplateDef`、`AssembleOptions`、`ChartWarning`、`ChannelSemantics`。 +## 校验 + +让 Agent 编写 `ChartAssemblyInput` 的宿主可以在渲染前先校验输入。`validateChart` +不会抛出异常；它会返回 assembler 产生的全部警告，并在输入无法编译时（未知图表类型、 +不支持的通道、不存在的字段、画布上限）返回一条 `assembly_failed` 错误。 +从 `flint-chart` 与 `flint-chart/validate` 再导出： + +```ts +import { validateChart } from 'flint-chart/validate'; + +const result = validateChart(input, 'vegalite'); +// { backend, chartType, valid, warnings, errors, computedSize? } +if (!result.valid) { + // 将 result.errors 反馈给 Agent +} +``` + +| 符号 | 用途 | +|--------|---------| +| `validateChart(input, backend, options?)` | 校验并装配；不抛出异常 | +| `validateChartInput(input, backend?, options?)` | 仅做结构检查；遇到第一个问题即抛出 | +| `validateSemanticTypes(semantic_types)` | 返回未在类型注册表中的标签（`validateChart` 以 `unknown_semantic_type` 警告报告） | +| `assembleForBackend(backend, input, options?)` | 装配并拆出 `_warnings` / `_width` / `_height` | +| `stripPrivateKeys(spec)` | 从 spec 中移除 Flint 的 `_` 前缀元数据 | + +`options.maxDataRows`（默认 100,000）与 `options.maxCanvasDim`（默认 4000）限制输入 +大小。要求内联的 `data.values` —— 请在校验前先把 `data.url` 解析为行数据。 + --- # §8 溢出与警告 @@ -288,6 +316,7 @@ vlGetTemplateChannels('Scatter Plot'); | `flint-chart/vegalite` | VL 模板与 `assembleVegaLite` | | `flint-chart/echarts` | ECharts 模板与 `assembleECharts` | | `flint-chart/chartjs` | Chart.js 模板与 `assembleChartjs` | +| `flint-chart/validate` | `validateChart` 与输入校验辅助函数 | | `flint-chart/test-data` | 图库生成器（`TEST_GENERATORS`） | --- diff --git a/packages/flint-js/README.md b/packages/flint-js/README.md index fd333b07..a7c19877 100644 --- a/packages/flint-js/README.md +++ b/packages/flint-js/README.md @@ -80,6 +80,7 @@ const xl = assembleExcel(input); // Native Excel chart artifact | `flint-chart/chartjs` | Chart.js backend | | `flint-chart/plotly` | Plotly backend | | `flint-chart/excel` | Native Excel / Office.js backend | +| `flint-chart/validate` | `validateChart` — per-problem input validation for agent loops | | `flint-chart/test-data` | Sample data generators used by the gallery and tests | | `flint-chart/gallery` | Curated example specs | diff --git a/packages/flint-js/package.json b/packages/flint-js/package.json index 28627964..a8aefdc6 100644 --- a/packages/flint-js/package.json +++ b/packages/flint-js/package.json @@ -70,6 +70,11 @@ "import": "./dist/image-charts/index.js", "require": "./dist/image-charts/index.cjs" }, + "./validate": { + "types": "./dist/validate/index.d.ts", + "import": "./dist/validate/index.js", + "require": "./dist/validate/index.cjs" + }, "./interactive": { "types": "./dist/interactive/index.d.ts", "import": "./dist/interactive/index.js", diff --git a/packages/flint-js/src/core/index.ts b/packages/flint-js/src/core/index.ts index da968a5e..f7585510 100644 --- a/packages/flint-js/src/core/index.ts +++ b/packages/flint-js/src/core/index.ts @@ -196,6 +196,7 @@ export { resolveStackable, resolveSortDirection, } from './field-semantics'; +export { isRegistered, getRegisteredTypes } from './type-registry'; // ThemeSpec: public visual-system vocabulary and chart-specific grounding export { diff --git a/packages/flint-js/src/index.ts b/packages/flint-js/src/index.ts index eeb753d1..1112f31d 100644 --- a/packages/flint-js/src/index.ts +++ b/packages/flint-js/src/index.ts @@ -26,6 +26,11 @@ * ecTemplateDefs / ecGetTemplateDef / ecGetTemplateChannels * cjsTemplateDefs / cjsGetTemplateDef / cjsGetTemplateChannels * + * Validation (also available from 'flint-chart/validate'): + * validateChart(input, backend) — never throws; { valid, warnings, errors, computedSize } + * validateChartInput(input, backend) — throws on the first problem + * validateSemanticTypes(types) — unregistered semantic_types labels + * * Usage: * ```ts * import { assembleVegaLite } from 'flint-chart'; @@ -60,3 +65,6 @@ export * from './excel'; // Image-Charts backend: assembleImageCharts + hosted-image-URL artifact type export * from './image-charts'; + +// Validation: validateChart, validateChartInput, assembleForBackend +export * from './validate'; diff --git a/packages/flint-js/src/validate/index.ts b/packages/flint-js/src/validate/index.ts new file mode 100644 index 00000000..1be1c5d2 --- /dev/null +++ b/packages/flint-js/src/validate/index.ts @@ -0,0 +1,341 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * @module flint-chart/validate + * + * Backend-aware validation of {@link ChartAssemblyInput} for hosts that let an + * agent author chart inputs and need precise, per-problem feedback before + * anything renders. Pure JS — no file system or native dependencies. Inline + * `data.values` are required; hosts resolve `data.url` to rows themselves. + * + * ```ts + * import { validateChart } from 'flint-chart/validate'; + * + * const result = validateChart(input, 'vegalite'); + * if (!result.valid) console.log(result.errors); + * ``` + */ + +import type { + ChartAssemblyInput, + ChartEncoding, + ChartTemplateDef, + ChartWarning, +} from '../core/types'; +import { isRegistered } from '../core/type-registry'; +import { toTypeString } from '../core/field-semantics'; +import { assembleVegaLite } from '../vegalite/assemble'; +import { vlGetTemplateDef } from '../vegalite/templates'; +import { assembleECharts } from '../echarts/assemble'; +import { ecGetTemplateDef } from '../echarts/templates'; +import { assembleChartjs } from '../chartjs/assemble'; +import { cjsGetTemplateDef } from '../chartjs/templates'; +import { assemblePlotly } from '../plotly/assemble'; +import { plGetTemplateDef } from '../plotly/templates'; + +/** Backends whose inputs can be validated and assembled. */ +export type ValidationBackend = 'vegalite' | 'echarts' | 'chartjs' | 'plotly'; + +export const VALIDATION_BACKENDS: readonly ValidationBackend[] = [ + 'vegalite', + 'echarts', + 'chartjs', + 'plotly', +]; + +/** Default cap on inline data rows. */ +export const DEFAULT_MAX_DATA_ROWS = 100_000; + +/** Default cap on `baseSize` / `canvasSize` dimensions in pixels. */ +export const DEFAULT_MAX_CANVAS_DIM = 4000; + +export interface ValidateChartOptions { + /** Maximum number of inline data rows accepted. Default: 100,000. */ + maxDataRows?: number; + /** Maximum `baseSize` / `canvasSize` dimension in pixels. Default: 4000. */ + maxCanvasDim?: number; +} + +export interface AssembleResult { + /** The backend-native spec (still carrying Flint's private `_`-keys). */ + spec: any; + /** Warnings emitted by the assembler. */ + warnings: ChartWarning[]; + /** Computed subplot width from the stretch model, if present. */ + width?: number; + /** Computed subplot height from the stretch model, if present. */ + height?: number; +} + +export interface ValidateResult { + backend: ValidationBackend; + chartType: string; + /** True when assembly succeeded with no error-severity warnings. */ + valid: boolean; + /** All warnings (info/warning/error) emitted during validation and assembly. */ + warnings: ChartWarning[]; + /** Error-severity warnings plus any thrown assembly failure. */ + errors: ChartWarning[]; + /** Computed layout size from Flint's stretch model, if available. */ + computedSize?: { width: number; height: number }; +} + +export interface SemanticTypeIssue { + field: string; + semanticType: string; +} + +const ASSEMBLERS: Record any> = { + vegalite: assembleVegaLite, + echarts: assembleECharts, + chartjs: assembleChartjs, + plotly: assemblePlotly, +}; + +const TEMPLATE_LOOKUP: Record< + ValidationBackend, + (chartType: string) => ChartTemplateDef | undefined +> = { + vegalite: vlGetTemplateDef, + echarts: ecGetTemplateDef, + chartjs: cjsGetTemplateDef, + plotly: plGetTemplateDef, +}; + +/** + * Validate the shape of a {@link ChartAssemblyInput} before it reaches an + * assembler: data presence and caps, `chartType`, encodings against the + * backend's template (channel support, required channels, field existence), + * and canvas caps. Throws on the first problem found. When `backend` is given + * and the chart type is unknown to that backend, encoding checks are skipped + * so the assembler reports the unknown type. + */ +export function validateChartInput( + input: ChartAssemblyInput, + backend?: ValidationBackend, + options: ValidateChartOptions = {}, +): void { + const maxDataRows = options.maxDataRows ?? DEFAULT_MAX_DATA_ROWS; + const maxCanvasDim = options.maxCanvasDim ?? DEFAULT_MAX_CANVAS_DIM; + + if (input == null || typeof input !== 'object') { + throw new Error('input must be a ChartAssemblyInput object'); + } + const data: any = (input as any).data; + if (data == null || typeof data !== 'object') { + throw new Error('input.data is required (provide { values: [...] })'); + } + if (!Array.isArray(data.values)) { + throw new Error( + 'input.data must provide inline values (resolve data.url to rows before validating)', + ); + } + if (data.values.length > maxDataRows) { + throw new Error( + `input.data.values has ${data.values.length} rows, exceeding the limit of ${maxDataRows}`, + ); + } + const cs: any = (input as any).chart_spec; + if (cs == null || typeof cs !== 'object' || typeof cs.chartType !== 'string') { + throw new Error('input.chart_spec.chartType is required'); + } + if (data.values.length === 0) { + throw new Error('input.data.values must contain at least one row'); + } + validateChartSpec(cs, data.values, backend); + for (const field of ['baseSize', 'canvasSize'] as const) { + const size = cs[field]; + if (size) { + if ( + (typeof size.width === 'number' && size.width > maxCanvasDim) || + (typeof size.height === 'number' && size.height > maxCanvasDim) + ) { + throw new Error( + `chart_spec.${field} exceeds the maximum dimension of ${maxCanvasDim}px`, + ); + } + } + } +} + +function validateChartSpec( + cs: any, + rows: Record[], + backend?: ValidationBackend, +): void { + const encodings = cs.encodings; + if (encodings == null || typeof encodings !== 'object' || Array.isArray(encodings)) { + throw new Error('input.chart_spec.encodings must be a channel-to-encoding object'); + } + + const entries = Object.entries(encodings); + if (entries.length === 0) { + throw new Error('input.chart_spec.encodings must bind at least one channel'); + } + + const template = backend ? TEMPLATE_LOOKUP[backend]?.(cs.chartType) : undefined; + if (backend && !template) return; + + if (template) { + const allowed = new Set(template.channels ?? []); + for (const [channel] of entries) { + if (!allowed.has(channel)) { + throw new Error( + `chart_spec.encodings.${channel} is not supported by ${cs.chartType} for ${backend}`, + ); + } + } + + for (const channel of requiredChannels(template)) { + if (!hasEncodingBinding(encodings[channel])) { + throw new Error(`chart_spec.encodings.${channel} is required for ${cs.chartType}`); + } + } + } + + const dataFields = new Set(rows.flatMap((row) => Object.keys(row))); + for (const [channel, encoding] of entries) { + for (const field of encodingFields(encoding)) { + if (!dataFields.has(field)) { + throw new Error( + `chart_spec.encodings.${channel}.field "${field}" does not exist in data.values`, + ); + } + } + } +} + +function requiredChannels(template: ChartTemplateDef): string[] { + const channels = template.channels ?? []; + if (channels.includes('x') && channels.includes('y')) return ['x', 'y']; + if (template.chart === 'KPI Card') return ['metric', 'value']; + return []; +} + +function hasEncodingBinding(value: unknown): boolean { + if (typeof value === 'string') return value.trim().length > 0; + if (Array.isArray(value)) return value.some(hasEncodingBinding); + if (value && typeof value === 'object') { + const encoding = value as ChartEncoding; + return ( + (typeof encoding.field === 'string' && encoding.field.trim().length > 0) || + encoding.aggregate === 'count' + ); + } + return false; +} + +function encodingFields(value: unknown): string[] { + if (typeof value === 'string') return value.trim() ? [value] : []; + if (Array.isArray(value)) return value.flatMap(encodingFields); + if (value && typeof value === 'object') { + const field = (value as ChartEncoding).field; + return typeof field === 'string' && field.trim() ? [field] : []; + } + return []; +} + +/** + * Report `semantic_types` labels that are not in Flint's type registry. + * Unregistered labels are not an error — assembly falls back to inferring + * from the data — but a host that expects the registry to be honored can use + * this to catch typos and drift. + */ +export function validateSemanticTypes( + semanticTypes: ChartAssemblyInput['semantic_types'] | undefined, +): SemanticTypeIssue[] { + if (semanticTypes == null || typeof semanticTypes !== 'object') return []; + const issues: SemanticTypeIssue[] = []; + for (const [field, annotation] of Object.entries(semanticTypes)) { + const semanticType = toTypeString(annotation); + if (!semanticType || !isRegistered(semanticType)) { + issues.push({ field, semanticType }); + } + } + return issues; +} + +/** + * Validate and assemble a Flint spec for one backend, splitting out Flint's + * private metadata (`_warnings`, `_width`, `_height`). The returned `spec` is + * left untouched so callers can choose to expose or strip the private keys + * (see {@link stripPrivateKeys}). Throws on validation or assembly failure. + */ +export function assembleForBackend( + backend: ValidationBackend, + input: ChartAssemblyInput, + options: ValidateChartOptions = {}, +): AssembleResult { + const assemble = ASSEMBLERS[backend]; + if (!assemble) { + throw new Error(`unknown backend: ${backend}`); + } + validateChartInput(input, backend, options); + const spec = assemble(input); + const warnings: ChartWarning[] = Array.isArray(spec?._warnings) ? spec._warnings : []; + const width = typeof spec?._width === 'number' ? spec._width : undefined; + const height = typeof spec?._height === 'number' ? spec._height : undefined; + return { spec, warnings, width, height }; +} + +/** + * Validate a {@link ChartAssemblyInput} for a backend: report warnings/errors, + * applicability, and the computed layout size. Never throws — validation and + * assembly failures are surfaced as an error entry. Unregistered + * `semantic_types` labels are reported as `unknown_semantic_type` warnings and + * do not affect `valid`. + */ +export function validateChart( + input: ChartAssemblyInput, + backend: ValidationBackend, + options: ValidateChartOptions = {}, +): ValidateResult { + const chartType = input?.chart_spec?.chartType ?? '(unknown)'; + const semanticTypeWarnings: ChartWarning[] = validateSemanticTypes( + input?.semantic_types, + ).map(({ field, semanticType }) => ({ + severity: 'warning', + code: 'unknown_semantic_type', + message: `semantic_types.${field} "${semanticType}" is not a registered semantic type; the field's type will be inferred from the data`, + field, + })); + try { + const { warnings, width, height } = assembleForBackend(backend, input, options); + const all = [...warnings, ...semanticTypeWarnings]; + const errors = all.filter((w) => w.severity === 'error'); + return { + backend, + chartType, + valid: errors.length === 0, + warnings: all, + errors, + computedSize: + typeof width === 'number' && typeof height === 'number' + ? { width, height } + : undefined, + }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + return { + backend, + chartType, + valid: false, + warnings: semanticTypeWarnings, + errors: [{ severity: 'error', code: 'assembly_failed', message }], + }; + } +} + +/** + * Remove Flint's private `_`-prefixed annotation keys from a top-level spec + * object so it is render-ready and safe to surface to callers. + */ +export function stripPrivateKeys>(spec: T): T { + for (const key of Object.keys(spec)) { + if (key.startsWith('_')) { + delete (spec as Record)[key]; + } + } + return spec; +} diff --git a/packages/flint-js/tests/validate.test.ts b/packages/flint-js/tests/validate.test.ts new file mode 100644 index 00000000..04196f9b --- /dev/null +++ b/packages/flint-js/tests/validate.test.ts @@ -0,0 +1,168 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, it, expect } from 'vitest'; +import { + VALIDATION_BACKENDS, + assembleForBackend, + stripPrivateKeys, + validateChart, + validateChartInput, + validateSemanticTypes, +} from '../src/validate'; +import type { ChartAssemblyInput } from '../src'; + +const barChart: ChartAssemblyInput = { + data: { + values: [ + { region: 'East', revenue: 120 }, + { region: 'West', revenue: 90 }, + { region: 'North', revenue: 150 }, + ], + }, + semantic_types: { region: 'Region', revenue: 'Amount' }, + chart_spec: { + chartType: 'Bar Chart', + encodings: { x: { field: 'region' }, y: { field: 'revenue' } }, + }, +}; + +function withSpec(chart_spec: Partial): ChartAssemblyInput { + return { ...barChart, chart_spec: { ...barChart.chart_spec, ...chart_spec } }; +} + +describe('validateChart', () => { + it.each(VALIDATION_BACKENDS)('accepts a valid bar chart for %s', (backend) => { + const result = validateChart(barChart, backend); + expect(result.backend).toBe(backend); + expect(result.chartType).toBe('Bar Chart'); + expect(result.valid).toBe(true); + expect(result.errors).toEqual([]); + }); + + it('reports the computed layout size', () => { + const result = validateChart(barChart, 'vegalite'); + expect(result.computedSize?.width).toBeGreaterThan(0); + expect(result.computedSize?.height).toBeGreaterThan(0); + }); + + it('flags an unknown chart type as invalid without throwing', () => { + const result = validateChart(withSpec({ chartType: 'Not A Real Chart' }), 'vegalite'); + expect(result.valid).toBe(false); + expect(result.chartType).toBe('Not A Real Chart'); + expect(result.errors[0].code).toBe('assembly_failed'); + expect(result.errors[0].message).toMatch(/Unknown chart type/); + }); + + it('flags a nonexistent field', () => { + const result = validateChart( + withSpec({ encodings: { x: { field: 'missing' }, y: { field: 'revenue' } } }), + 'vegalite', + ); + expect(result.valid).toBe(false); + expect(result.errors[0].message).toContain('"missing" does not exist in data.values'); + }); + + it('flags an unsupported channel', () => { + const result = validateChart( + withSpec({ + encodings: { x: { field: 'region' }, y: { field: 'revenue' }, banana: { field: 'region' } }, + }), + 'echarts', + ); + expect(result.valid).toBe(false); + expect(result.errors[0].message).toContain('encodings.banana is not supported by Bar Chart for echarts'); + }); + + it('flags a canvas that exceeds the cap', () => { + const result = validateChart(withSpec({ canvasSize: { width: 5000, height: 300 } }), 'vegalite'); + expect(result.valid).toBe(false); + expect(result.errors[0].message).toContain('maximum dimension of 4000px'); + + const relaxed = validateChart(withSpec({ canvasSize: { width: 5000, height: 300 } }), 'vegalite', { + maxCanvasDim: 8000, + }); + expect(relaxed.valid).toBe(true); + }); + + it('rejects malformed input without throwing', () => { + for (const input of [null, undefined, 42, {}, { data: {} }, { data: { url: 'x.csv' } }]) { + const result = validateChart(input as any, 'vegalite'); + expect(result.valid).toBe(false); + expect(result.chartType).toBe('(unknown)'); + expect(result.errors).toHaveLength(1); + } + }); + + it('surfaces unregistered semantic_types as warnings, not errors', () => { + const result = validateChart( + { ...barChart, semantic_types: { region: 'Region', revenue: 'Dollarz' } }, + 'vegalite', + ); + expect(result.valid).toBe(true); + const warning = result.warnings.find((w) => w.code === 'unknown_semantic_type'); + expect(warning?.severity).toBe('warning'); + expect(warning?.field).toBe('revenue'); + expect(warning?.message).toContain('"Dollarz"'); + }); +}); + +describe('validateChartInput', () => { + it('throws on the first problem', () => { + expect(() => validateChartInput(withSpec({ encodings: {} }), 'vegalite')).toThrow( + /must bind at least one channel/, + ); + expect(() => + validateChartInput({ ...barChart, data: { values: [] } }, 'vegalite'), + ).toThrow(/at least one row/); + expect(() => + validateChartInput(barChart, 'vegalite', { maxDataRows: 2 }), + ).toThrow(/exceeding the limit of 2/); + }); + + it('requires x and y for cartesian templates', () => { + expect(() => + validateChartInput(withSpec({ encodings: { x: { field: 'region' } } }), 'vegalite'), + ).toThrow(/encodings\.y is required for Bar Chart/); + }); + + it('checks fields without a backend', () => { + expect(() => + validateChartInput(withSpec({ encodings: { x: 'nope', y: 'revenue' } })), + ).toThrow(/"nope" does not exist/); + expect(() => validateChartInput(barChart)).not.toThrow(); + }); +}); + +describe('validateSemanticTypes', () => { + it('returns only unregistered labels', () => { + expect( + validateSemanticTypes({ + a: 'Amount', + b: { semanticType: 'Percentage' }, + c: 'NotAType', + d: { semanticType: '' }, + }), + ).toEqual([ + { field: 'c', semanticType: 'NotAType' }, + { field: 'd', semanticType: '' }, + ]); + expect(validateSemanticTypes(undefined)).toEqual([]); + }); +}); + +describe('assembleForBackend', () => { + it('splits Flint metadata out of the spec', () => { + const { spec, warnings, width, height } = assembleForBackend('echarts', barChart); + expect(Array.isArray(warnings)).toBe(true); + expect(typeof width).toBe('number'); + expect(typeof height).toBe('number'); + expect(Object.keys(spec).some((k) => k.startsWith('_'))).toBe(true); + stripPrivateKeys(spec); + expect(Object.keys(spec).some((k) => k.startsWith('_'))).toBe(false); + }); + + it('rejects an unknown backend', () => { + expect(() => assembleForBackend('excel' as any, barChart)).toThrow(/unknown backend/); + }); +}); diff --git a/packages/flint-js/tsup.config.ts b/packages/flint-js/tsup.config.ts index 6b795ae6..96d76376 100644 --- a/packages/flint-js/tsup.config.ts +++ b/packages/flint-js/tsup.config.ts @@ -10,6 +10,7 @@ export default defineConfig({ 'plotly/index': 'src/plotly/index.ts', 'excel/index': 'src/excel/index.ts', 'image-charts/index': 'src/image-charts/index.ts', + 'validate/index': 'src/validate/index.ts', 'interactive/index': 'src/interactive/index.ts', 'vegalite/interactive': 'src/vegalite/interactive.ts', 'echarts/interactive': 'src/echarts/interactive.ts', diff --git a/packages/flint-mcp/src/render/assemble.ts b/packages/flint-mcp/src/render/assemble.ts index 0ab4d868..e59a200b 100644 --- a/packages/flint-mcp/src/render/assemble.ts +++ b/packages/flint-mcp/src/render/assemble.ts @@ -2,16 +2,14 @@ // Licensed under the MIT License. import { - assembleVegaLite, - assembleECharts, - assembleChartjs, + assembleForBackend as coreAssembleForBackend, + validateChartInput, + stripPrivateKeys, + DEFAULT_MAX_CANVAS_DIM, + DEFAULT_MAX_DATA_ROWS, + type AssembleResult, type ChartAssemblyInput, - type ChartEncoding, - type ChartTemplateDef, - type ChartWarning, - vlGetTemplateDef, - ecGetTemplateDef, - cjsGetTemplateDef, + type ValidateChartOptions, } from 'flint-chart'; import { resolveDataSource, @@ -19,38 +17,19 @@ import { } from './data-source.js'; import type { RenderBackend } from './types.js'; +export { stripPrivateKeys, type AssembleResult }; + /** Maximum number of inline data rows accepted (DoS guard). */ -export const MAX_DATA_ROWS = 100_000; +export const MAX_DATA_ROWS = DEFAULT_MAX_DATA_ROWS; /** Maximum canvas dimension in pixels the host will honor (DoS guard). */ -export const MAX_CANVAS_DIM = 4000; - -const ASSEMBLERS: Record< - RenderBackend, - (input: ChartAssemblyInput) => any -> = { - vegalite: assembleVegaLite, - echarts: assembleECharts, - chartjs: assembleChartjs, -}; +export const MAX_CANVAS_DIM = DEFAULT_MAX_CANVAS_DIM; -const TEMPLATE_LOOKUP: Record ChartTemplateDef | undefined> = { - vegalite: vlGetTemplateDef, - echarts: ecGetTemplateDef, - chartjs: cjsGetTemplateDef, +const CAPS: ValidateChartOptions = { + maxDataRows: MAX_DATA_ROWS, + maxCanvasDim: MAX_CANVAS_DIM, }; -export interface AssembleResult { - /** The backend-native spec (still carrying Flint's private `_`-keys). */ - spec: any; - /** Warnings emitted by the assembler. */ - warnings: ChartWarning[]; - /** Computed subplot width from the stretch model, if present. */ - width?: number; - /** Computed subplot height from the stretch model, if present. */ - height?: number; -} - /** * Validate caller-supplied input before it reaches an assembler. Inline rows * pass through directly. Local `data.url` references are read unless @@ -60,14 +39,13 @@ export function validateInput( input: ChartAssemblyInput, options: DataSourceOptions = {}, ): void { - prepareInput(input, options); + validateChartInput(resolveInput(input, options), undefined, CAPS); } -/** Resolve data references and validate caller-supplied input. */ -export function prepareInput( +/** Resolve `data.url` to inline rows so the core validator can see them. */ +export function resolveInput( input: ChartAssemblyInput, options: DataSourceOptions = {}, - backend?: RenderBackend, ): ChartAssemblyInput { if (input == null || typeof input !== 'object') { throw new Error('input must be a ChartAssemblyInput object'); @@ -76,148 +54,26 @@ export function prepareInput( ...options, maxDataRows: MAX_DATA_ROWS, }); - const resolvedData: any = (resolvedInput as any).data; - if (resolvedData == null || typeof resolvedData !== 'object') { - throw new Error('input.data is required (provide { values: [...] })'); - } - if (!Array.isArray(resolvedData.values)) { + // The core message assumes a host that never sees data.url; ours does. + const data: any = (resolvedInput as any).data; + if (data && typeof data === 'object' && !Array.isArray(data.values)) { throw new Error( 'input.data must provide inline values or a readable local data.url', ); } - if (resolvedData.values.length > MAX_DATA_ROWS) { - throw new Error( - `input.data.values has ${resolvedData.values.length} rows, exceeding the limit of ${MAX_DATA_ROWS}`, - ); - } - const cs: any = (resolvedInput as any).chart_spec; - if (cs == null || typeof cs !== 'object' || typeof cs.chartType !== 'string') { - throw new Error('input.chart_spec.chartType is required'); - } - if (resolvedData.values.length === 0) { - throw new Error('input.data.values must contain at least one row'); - } - validateChartSpec(cs, resolvedData.values, backend); - for (const field of ['baseSize', 'canvasSize'] as const) { - const size = cs[field]; - if (size) { - if ( - (typeof size.width === 'number' && size.width > MAX_CANVAS_DIM) || - (typeof size.height === 'number' && size.height > MAX_CANVAS_DIM) - ) { - throw new Error( - `chart_spec.${field} exceeds the maximum dimension of ${MAX_CANVAS_DIM}px`, - ); - } - } - } return resolvedInput; } -function validateChartSpec(cs: any, rows: Record[], backend?: RenderBackend): void { - const encodings = cs.encodings; - if (encodings == null || typeof encodings !== 'object' || Array.isArray(encodings)) { - throw new Error('input.chart_spec.encodings must be a channel-to-encoding object'); - } - - const entries = Object.entries(encodings); - if (entries.length === 0) { - throw new Error('input.chart_spec.encodings must bind at least one channel'); - } - - const template = backend ? TEMPLATE_LOOKUP[backend]?.(cs.chartType) : undefined; - if (backend && !template) return; - - if (template) { - const allowed = new Set(template.channels ?? []); - for (const [channel] of entries) { - if (!allowed.has(channel)) { - throw new Error( - `chart_spec.encodings.${channel} is not supported by ${cs.chartType} for ${backend}`, - ); - } - } - - for (const channel of requiredChannels(template)) { - if (!hasEncodingBinding(encodings[channel])) { - throw new Error(`chart_spec.encodings.${channel} is required for ${cs.chartType}`); - } - } - } - - const dataFields = new Set(rows.flatMap((row) => Object.keys(row))); - for (const [channel, encoding] of entries) { - for (const field of encodingFields(encoding)) { - if (!dataFields.has(field)) { - throw new Error(`chart_spec.encodings.${channel}.field "${field}" does not exist in data.values`); - } - } - } -} - -function requiredChannels(template: ChartTemplateDef): string[] { - const channels = template.channels ?? []; - if (channels.includes('x') && channels.includes('y')) return ['x', 'y']; - if (template.chart === 'KPI Card') return ['metric', 'value']; - return []; -} - -function hasEncodingBinding(value: unknown): boolean { - if (typeof value === 'string') return value.trim().length > 0; - if (Array.isArray(value)) return value.some(hasEncodingBinding); - if (value && typeof value === 'object') { - const encoding = value as ChartEncoding; - return ( - (typeof encoding.field === 'string' && encoding.field.trim().length > 0) || - encoding.aggregate === 'count' - ); - } - return false; -} - -function encodingFields(value: unknown): string[] { - if (typeof value === 'string') return value.trim() ? [value] : []; - if (Array.isArray(value)) return value.flatMap(encodingFields); - if (value && typeof value === 'object') { - const field = (value as ChartEncoding).field; - return typeof field === 'string' && field.trim() ? [field] : []; - } - return []; -} - /** - * Assemble a Flint spec for one backend and split out Flint's private metadata - * (`_warnings`, `_width`, `_height`). The returned `spec` is left untouched so - * callers can choose to expose or strip the private keys. + * Resolve `data.url`, then assemble a Flint spec for one backend and split out + * Flint's private metadata (`_warnings`, `_width`, `_height`). The returned + * `spec` is left untouched so callers can choose to expose or strip the + * private keys. */ export function assembleForBackend( backend: RenderBackend, input: ChartAssemblyInput, options: DataSourceOptions = {}, ): AssembleResult { - const assemble = ASSEMBLERS[backend]; - if (!assemble) { - throw new Error(`unknown backend: ${backend}`); - } - const resolvedInput = prepareInput(input, options, backend); - const spec = assemble(resolvedInput); - const warnings: ChartWarning[] = Array.isArray(spec?._warnings) - ? spec._warnings - : []; - const width = typeof spec?._width === 'number' ? spec._width : undefined; - const height = typeof spec?._height === 'number' ? spec._height : undefined; - return { spec, warnings, width, height }; -} - -/** - * Remove Flint's private `_`-prefixed annotation keys from a top-level spec - * object so it is render-ready and safe to surface to callers. - */ -export function stripPrivateKeys>(spec: T): T { - for (const key of Object.keys(spec)) { - if (key.startsWith('_')) { - delete (spec as Record)[key]; - } - } - return spec; + return coreAssembleForBackend(backend, resolveInput(input, options), CAPS); } diff --git a/packages/flint-mcp/src/tools/validate.ts b/packages/flint-mcp/src/tools/validate.ts index 83479769..e0dff486 100644 --- a/packages/flint-mcp/src/tools/validate.ts +++ b/packages/flint-mcp/src/tools/validate.ts @@ -1,57 +1,47 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import type { ChartAssemblyInput, ChartWarning } from 'flint-chart'; -import { assembleForBackend } from '../render/assemble.js'; +import { + validateChart as coreValidateChart, + type ChartAssemblyInput, + type ValidateResult as CoreValidateResult, +} from 'flint-chart'; +import { MAX_CANVAS_DIM, MAX_DATA_ROWS, resolveInput } from '../render/assemble.js'; import type { DataSourceOptions } from '../render/data-source.js'; import type { RenderBackend } from '../render/types.js'; -export interface ValidateResult { +export interface ValidateResult extends CoreValidateResult { backend: RenderBackend; - chartType: string; - /** True when assembly succeeded with no error-severity warnings. */ - valid: boolean; - /** All warnings (info/warning/error) emitted during assembly. */ - warnings: ChartWarning[]; - /** Error-severity warnings plus any thrown assembly failure. */ - errors: ChartWarning[]; - /** Computed layout size from Flint's stretch model, if available. */ - computedSize?: { width: number; height: number }; } /** - * Validate a {@link ChartAssemblyInput} for a backend: report warnings/errors, - * applicability, and the computed layout size. Never throws — assembly failures - * are surfaced as an error entry. Pure JS — no native dependencies. + * Resolve `data.url`, then validate a {@link ChartAssemblyInput} for a backend + * via `flint-chart`'s `validateChart`. Never throws — data-source and assembly + * failures are surfaced as an error entry. */ export function validateChart( input: ChartAssemblyInput, backend: RenderBackend, options: DataSourceOptions = {}, ): ValidateResult { - const chartType = input?.chart_spec?.chartType ?? '(unknown)'; + let resolvedInput: ChartAssemblyInput; try { - const { warnings, width, height } = assembleForBackend(backend, input, options); - const errors = warnings.filter((w) => w.severity === 'error'); - return { - backend, - chartType, - valid: errors.length === 0, - warnings, - errors, - computedSize: - typeof width === 'number' && typeof height === 'number' - ? { width, height } - : undefined, - }; + resolvedInput = resolveInput(input, options); } catch (err) { const message = err instanceof Error ? err.message : String(err); return { backend, - chartType, + chartType: input?.chart_spec?.chartType ?? '(unknown)', valid: false, warnings: [], errors: [{ severity: 'error', code: 'assembly_failed', message }], }; } + return { + ...coreValidateChart(resolvedInput, backend, { + maxDataRows: MAX_DATA_ROWS, + maxCanvasDim: MAX_CANVAS_DIM, + }), + backend, + }; } From a0bb5d5ff5ddff7c793faffa2d8999147af45868 Mon Sep 17 00:00:00 2001 From: taoche Date: Fri, 4 Sep 2026 16:07:49 +0800 Subject: [PATCH 2/4] refactor(validate): remove duplicated checks between core and mcp - validateSemanticTypes returns ChartWarning[] directly instead of a separate issue shape that validateChart then re-mapped - drop the MCP-side re-check of data.values that only existed to word the error differently; core message is now host-neutral - MCP reuses core ValidateResult and a single CAPS constant Co-Authored-By: Claude Code --- docs/api-reference.md | 2 +- docs/zh-CN/api-reference.md | 2 +- packages/flint-js/src/validate/index.ts | 43 +++++++++-------------- packages/flint-js/tests/validate.test.ts | 6 ++-- packages/flint-mcp/src/render/assemble.ts | 17 ++------- packages/flint-mcp/src/tools/validate.ts | 16 +++------ 6 files changed, 29 insertions(+), 57 deletions(-) diff --git a/docs/api-reference.md b/docs/api-reference.md index 52aac166..538d3f33 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -314,7 +314,7 @@ if (!result.valid) { |--------|---------| | `validateChart(input, backend, options?)` | Validate and assemble; never throws | | `validateChartInput(input, backend?, options?)` | Shape checks only; throws on the first problem | -| `validateSemanticTypes(semantic_types)` | Labels not in the type registry (reported by `validateChart` as `unknown_semantic_type` warnings) | +| `validateSemanticTypes(semantic_types)` | `unknown_semantic_type` warnings for labels not in the type registry (also included by `validateChart`) | | `assembleForBackend(backend, input, options?)` | Assemble and split out `_warnings` / `_width` / `_height` | | `stripPrivateKeys(spec)` | Remove Flint's `_`-prefixed metadata from a spec | diff --git a/docs/zh-CN/api-reference.md b/docs/zh-CN/api-reference.md index 3eb4e28f..2319429c 100644 --- a/docs/zh-CN/api-reference.md +++ b/docs/zh-CN/api-reference.md @@ -276,7 +276,7 @@ if (!result.valid) { |--------|---------| | `validateChart(input, backend, options?)` | 校验并装配；不抛出异常 | | `validateChartInput(input, backend?, options?)` | 仅做结构检查；遇到第一个问题即抛出 | -| `validateSemanticTypes(semantic_types)` | 返回未在类型注册表中的标签（`validateChart` 以 `unknown_semantic_type` 警告报告） | +| `validateSemanticTypes(semantic_types)` | 对未在类型注册表中的标签返回 `unknown_semantic_type` 警告（`validateChart` 也会包含这些警告） | | `assembleForBackend(backend, input, options?)` | 装配并拆出 `_warnings` / `_width` / `_height` | | `stripPrivateKeys(spec)` | 从 spec 中移除 Flint 的 `_` 前缀元数据 | diff --git a/packages/flint-js/src/validate/index.ts b/packages/flint-js/src/validate/index.ts index 1be1c5d2..dd368e9b 100644 --- a/packages/flint-js/src/validate/index.ts +++ b/packages/flint-js/src/validate/index.ts @@ -81,11 +81,6 @@ export interface ValidateResult { computedSize?: { width: number; height: number }; } -export interface SemanticTypeIssue { - field: string; - semanticType: string; -} - const ASSEMBLERS: Record any> = { vegalite: assembleVegaLite, echarts: assembleECharts, @@ -127,9 +122,7 @@ export function validateChartInput( throw new Error('input.data is required (provide { values: [...] })'); } if (!Array.isArray(data.values)) { - throw new Error( - 'input.data must provide inline values (resolve data.url to rows before validating)', - ); + throw new Error('input.data must provide inline values'); } if (data.values.length > maxDataRows) { throw new Error( @@ -237,23 +230,28 @@ function encodingFields(value: unknown): string[] { } /** - * Report `semantic_types` labels that are not in Flint's type registry. - * Unregistered labels are not an error — assembly falls back to inferring - * from the data — but a host that expects the registry to be honored can use - * this to catch typos and drift. + * Report `semantic_types` labels that are not in Flint's type registry as + * `unknown_semantic_type` warnings. Unregistered labels are not an error — + * assembly falls back to inferring from the data — but a host that expects + * the registry to be honored can use this to catch typos and drift. */ export function validateSemanticTypes( semanticTypes: ChartAssemblyInput['semantic_types'] | undefined, -): SemanticTypeIssue[] { +): ChartWarning[] { if (semanticTypes == null || typeof semanticTypes !== 'object') return []; - const issues: SemanticTypeIssue[] = []; + const warnings: ChartWarning[] = []; for (const [field, annotation] of Object.entries(semanticTypes)) { const semanticType = toTypeString(annotation); if (!semanticType || !isRegistered(semanticType)) { - issues.push({ field, semanticType }); + warnings.push({ + severity: 'warning', + code: 'unknown_semantic_type', + message: `semantic_types.${field} "${semanticType}" is not a registered semantic type; the field's type will be inferred from the data`, + field, + }); } } - return issues; + return warnings; } /** @@ -283,8 +281,8 @@ export function assembleForBackend( * Validate a {@link ChartAssemblyInput} for a backend: report warnings/errors, * applicability, and the computed layout size. Never throws — validation and * assembly failures are surfaced as an error entry. Unregistered - * `semantic_types` labels are reported as `unknown_semantic_type` warnings and - * do not affect `valid`. + * `semantic_types` labels are included as warnings (see + * {@link validateSemanticTypes}) and do not affect `valid`. */ export function validateChart( input: ChartAssemblyInput, @@ -292,14 +290,7 @@ export function validateChart( options: ValidateChartOptions = {}, ): ValidateResult { const chartType = input?.chart_spec?.chartType ?? '(unknown)'; - const semanticTypeWarnings: ChartWarning[] = validateSemanticTypes( - input?.semantic_types, - ).map(({ field, semanticType }) => ({ - severity: 'warning', - code: 'unknown_semantic_type', - message: `semantic_types.${field} "${semanticType}" is not a registered semantic type; the field's type will be inferred from the data`, - field, - })); + const semanticTypeWarnings = validateSemanticTypes(input?.semantic_types); try { const { warnings, width, height } = assembleForBackend(backend, input, options); const all = [...warnings, ...semanticTypeWarnings]; diff --git a/packages/flint-js/tests/validate.test.ts b/packages/flint-js/tests/validate.test.ts index 04196f9b..c580fe34 100644 --- a/packages/flint-js/tests/validate.test.ts +++ b/packages/flint-js/tests/validate.test.ts @@ -143,9 +143,9 @@ describe('validateSemanticTypes', () => { c: 'NotAType', d: { semanticType: '' }, }), - ).toEqual([ - { field: 'c', semanticType: 'NotAType' }, - { field: 'd', semanticType: '' }, + ).toMatchObject([ + { severity: 'warning', code: 'unknown_semantic_type', field: 'c' }, + { severity: 'warning', code: 'unknown_semantic_type', field: 'd' }, ]); expect(validateSemanticTypes(undefined)).toEqual([]); }); diff --git a/packages/flint-mcp/src/render/assemble.ts b/packages/flint-mcp/src/render/assemble.ts index e59a200b..7572f3fe 100644 --- a/packages/flint-mcp/src/render/assemble.ts +++ b/packages/flint-mcp/src/render/assemble.ts @@ -25,7 +25,7 @@ export const MAX_DATA_ROWS = DEFAULT_MAX_DATA_ROWS; /** Maximum canvas dimension in pixels the host will honor (DoS guard). */ export const MAX_CANVAS_DIM = DEFAULT_MAX_CANVAS_DIM; -const CAPS: ValidateChartOptions = { +export const CAPS: ValidateChartOptions = { maxDataRows: MAX_DATA_ROWS, maxCanvasDim: MAX_CANVAS_DIM, }; @@ -42,7 +42,7 @@ export function validateInput( validateChartInput(resolveInput(input, options), undefined, CAPS); } -/** Resolve `data.url` to inline rows so the core validator can see them. */ +/** Resolve `data.url` to inline rows so the core validator can see them. Throws on unreadable references. */ export function resolveInput( input: ChartAssemblyInput, options: DataSourceOptions = {}, @@ -50,18 +50,7 @@ export function resolveInput( if (input == null || typeof input !== 'object') { throw new Error('input must be a ChartAssemblyInput object'); } - const resolvedInput = resolveDataSource(input, { - ...options, - maxDataRows: MAX_DATA_ROWS, - }); - // The core message assumes a host that never sees data.url; ours does. - const data: any = (resolvedInput as any).data; - if (data && typeof data === 'object' && !Array.isArray(data.values)) { - throw new Error( - 'input.data must provide inline values or a readable local data.url', - ); - } - return resolvedInput; + return resolveDataSource(input, { ...options, maxDataRows: MAX_DATA_ROWS }); } /** diff --git a/packages/flint-mcp/src/tools/validate.ts b/packages/flint-mcp/src/tools/validate.ts index e0dff486..67b92b6b 100644 --- a/packages/flint-mcp/src/tools/validate.ts +++ b/packages/flint-mcp/src/tools/validate.ts @@ -4,15 +4,13 @@ import { validateChart as coreValidateChart, type ChartAssemblyInput, - type ValidateResult as CoreValidateResult, + type ValidateResult, } from 'flint-chart'; -import { MAX_CANVAS_DIM, MAX_DATA_ROWS, resolveInput } from '../render/assemble.js'; +import { CAPS, resolveInput } from '../render/assemble.js'; import type { DataSourceOptions } from '../render/data-source.js'; import type { RenderBackend } from '../render/types.js'; -export interface ValidateResult extends CoreValidateResult { - backend: RenderBackend; -} +export type { ValidateResult }; /** * Resolve `data.url`, then validate a {@link ChartAssemblyInput} for a backend @@ -37,11 +35,5 @@ export function validateChart( errors: [{ severity: 'error', code: 'assembly_failed', message }], }; } - return { - ...coreValidateChart(resolvedInput, backend, { - maxDataRows: MAX_DATA_ROWS, - maxCanvasDim: MAX_CANVAS_DIM, - }), - backend, - }; + return coreValidateChart(resolvedInput, backend, CAPS); } From eb9603535364766f6a638d425817c54f755163c8 Mon Sep 17 00:00:00 2001 From: taoche Date: Fri, 4 Sep 2026 16:14:02 +0800 Subject: [PATCH 3/4] refactor(validate): split validateChartInput into typed steps validateData / validateEncodings / validateEncodingsAgainstTemplate / validateCanvasCaps replace one 45-line function and the `any` casts with an isRecord type guard. Tests now cover one scenario each, plus unknown backend, non-array values, and template skipping for unknown chart types. Co-Authored-By: Claude Code --- docs/api-reference.md | 1 + docs/zh-CN/api-reference.md | 1 + packages/flint-js/src/validate/index.ts | 128 ++++++++++++---------- packages/flint-js/tests/validate.test.ts | 112 +++++++++++-------- packages/flint-mcp/src/render/assemble.ts | 6 +- packages/flint-mcp/src/tools/validate.ts | 4 +- 6 files changed, 148 insertions(+), 104 deletions(-) diff --git a/docs/api-reference.md b/docs/api-reference.md index 538d3f33..52e7d9ad 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -317,6 +317,7 @@ if (!result.valid) { | `validateSemanticTypes(semantic_types)` | `unknown_semantic_type` warnings for labels not in the type registry (also included by `validateChart`) | | `assembleForBackend(backend, input, options?)` | Assemble and split out `_warnings` / `_width` / `_height` | | `stripPrivateKeys(spec)` | Remove Flint's `_`-prefixed metadata from a spec | +| `VALIDATION_BACKENDS` | Runtime list of accepted backends (`vegalite`, `echarts`, `chartjs`, `plotly`) | `options.maxDataRows` (default 100,000) and `options.maxCanvasDim` (default 4000) cap input size. Inline `data.values` are required — resolve `data.url` diff --git a/docs/zh-CN/api-reference.md b/docs/zh-CN/api-reference.md index 2319429c..3a967257 100644 --- a/docs/zh-CN/api-reference.md +++ b/docs/zh-CN/api-reference.md @@ -279,6 +279,7 @@ if (!result.valid) { | `validateSemanticTypes(semantic_types)` | 对未在类型注册表中的标签返回 `unknown_semantic_type` 警告（`validateChart` 也会包含这些警告） | | `assembleForBackend(backend, input, options?)` | 装配并拆出 `_warnings` / `_width` / `_height` | | `stripPrivateKeys(spec)` | 从 spec 中移除 Flint 的 `_` 前缀元数据 | +| `VALIDATION_BACKENDS` | 运行时可用的 backend 列表（`vegalite`、`echarts`、`chartjs`、`plotly`） | `options.maxDataRows`（默认 100,000）与 `options.maxCanvasDim`（默认 4000）限制输入 大小。要求内联的 `data.values` —— 请在校验前先把 `data.url` 解析为行数据。 diff --git a/packages/flint-js/src/validate/index.ts b/packages/flint-js/src/validate/index.ts index dd368e9b..ba6b584e 100644 --- a/packages/flint-js/src/validate/index.ts +++ b/packages/flint-js/src/validate/index.ts @@ -111,80 +111,59 @@ export function validateChartInput( backend?: ValidationBackend, options: ValidateChartOptions = {}, ): void { - const maxDataRows = options.maxDataRows ?? DEFAULT_MAX_DATA_ROWS; - const maxCanvasDim = options.maxCanvasDim ?? DEFAULT_MAX_CANVAS_DIM; - - if (input == null || typeof input !== 'object') { + if (!isRecord(input)) { throw new Error('input must be a ChartAssemblyInput object'); } - const data: any = (input as any).data; - if (data == null || typeof data !== 'object') { + const rows = validateData(input.data, options.maxDataRows ?? DEFAULT_MAX_DATA_ROWS); + const chartSpec = input.chart_spec; + if (!isRecord(chartSpec) || typeof chartSpec.chartType !== 'string') { + throw new Error('input.chart_spec.chartType is required'); + } + validateEncodings(chartSpec.chartType, chartSpec.encodings, rows, backend); + validateCanvasCaps(chartSpec, options.maxCanvasDim ?? DEFAULT_MAX_CANVAS_DIM); +} + +function isRecord(value: unknown): value is Record { + return value != null && typeof value === 'object' && !Array.isArray(value); +} + +function validateData(data: unknown, maxDataRows: number): Record[] { + if (!isRecord(data)) { throw new Error('input.data is required (provide { values: [...] })'); } - if (!Array.isArray(data.values)) { + const rows = data.values; + if (!Array.isArray(rows)) { throw new Error('input.data must provide inline values'); } - if (data.values.length > maxDataRows) { + if (rows.length > maxDataRows) { throw new Error( - `input.data.values has ${data.values.length} rows, exceeding the limit of ${maxDataRows}`, + `input.data.values has ${rows.length} rows, exceeding the limit of ${maxDataRows}`, ); } - const cs: any = (input as any).chart_spec; - if (cs == null || typeof cs !== 'object' || typeof cs.chartType !== 'string') { - throw new Error('input.chart_spec.chartType is required'); - } - if (data.values.length === 0) { + if (rows.length === 0) { throw new Error('input.data.values must contain at least one row'); } - validateChartSpec(cs, data.values, backend); - for (const field of ['baseSize', 'canvasSize'] as const) { - const size = cs[field]; - if (size) { - if ( - (typeof size.width === 'number' && size.width > maxCanvasDim) || - (typeof size.height === 'number' && size.height > maxCanvasDim) - ) { - throw new Error( - `chart_spec.${field} exceeds the maximum dimension of ${maxCanvasDim}px`, - ); - } - } - } + return rows; } -function validateChartSpec( - cs: any, +function validateEncodings( + chartType: string, + encodings: unknown, rows: Record[], backend?: ValidationBackend, ): void { - const encodings = cs.encodings; - if (encodings == null || typeof encodings !== 'object' || Array.isArray(encodings)) { + if (!isRecord(encodings)) { throw new Error('input.chart_spec.encodings must be a channel-to-encoding object'); } - const entries = Object.entries(encodings); if (entries.length === 0) { throw new Error('input.chart_spec.encodings must bind at least one channel'); } - const template = backend ? TEMPLATE_LOOKUP[backend]?.(cs.chartType) : undefined; - if (backend && !template) return; - - if (template) { - const allowed = new Set(template.channels ?? []); - for (const [channel] of entries) { - if (!allowed.has(channel)) { - throw new Error( - `chart_spec.encodings.${channel} is not supported by ${cs.chartType} for ${backend}`, - ); - } - } - - for (const channel of requiredChannels(template)) { - if (!hasEncodingBinding(encodings[channel])) { - throw new Error(`chart_spec.encodings.${channel} is required for ${cs.chartType}`); - } - } + if (backend) { + const template = TEMPLATE_LOOKUP[backend]?.(chartType); + if (!template) return; + validateEncodingsAgainstTemplate(chartType, backend, template, encodings); } const dataFields = new Set(rows.flatMap((row) => Object.keys(row))); @@ -199,6 +178,43 @@ function validateChartSpec( } } +function validateEncodingsAgainstTemplate( + chartType: string, + backend: ValidationBackend, + template: ChartTemplateDef, + encodings: Record, +): void { + const allowed = new Set(template.channels ?? []); + for (const channel of Object.keys(encodings)) { + if (!allowed.has(channel)) { + throw new Error( + `chart_spec.encodings.${channel} is not supported by ${chartType} for ${backend}`, + ); + } + } + for (const channel of requiredChannels(template)) { + if (!hasEncodingBinding(encodings[channel])) { + throw new Error(`chart_spec.encodings.${channel} is required for ${chartType}`); + } + } +} + +function validateCanvasCaps(chartSpec: Record, maxCanvasDim: number): void { + for (const field of ['baseSize', 'canvasSize'] as const) { + const size = chartSpec[field]; + if (!isRecord(size)) continue; + const { width, height } = size; + if ( + (typeof width === 'number' && width > maxCanvasDim) || + (typeof height === 'number' && height > maxCanvasDim) + ) { + throw new Error( + `chart_spec.${field} exceeds the maximum dimension of ${maxCanvasDim}px`, + ); + } + } +} + function requiredChannels(template: ChartTemplateDef): string[] { const channels = template.channels ?? []; if (channels.includes('x') && channels.includes('y')) return ['x', 'y']; @@ -238,7 +254,7 @@ function encodingFields(value: unknown): string[] { export function validateSemanticTypes( semanticTypes: ChartAssemblyInput['semantic_types'] | undefined, ): ChartWarning[] { - if (semanticTypes == null || typeof semanticTypes !== 'object') return []; + if (!isRecord(semanticTypes)) return []; const warnings: ChartWarning[] = []; for (const [field, annotation] of Object.entries(semanticTypes)) { const semanticType = toTypeString(annotation); @@ -289,7 +305,9 @@ export function validateChart( backend: ValidationBackend, options: ValidateChartOptions = {}, ): ValidateResult { - const chartType = input?.chart_spec?.chartType ?? '(unknown)'; + const chartType = typeof input?.chart_spec?.chartType === 'string' + ? input.chart_spec.chartType + : '(unknown)'; const semanticTypeWarnings = validateSemanticTypes(input?.semantic_types); try { const { warnings, width, height } = assembleForBackend(backend, input, options); @@ -302,9 +320,7 @@ export function validateChart( warnings: all, errors, computedSize: - typeof width === 'number' && typeof height === 'number' - ? { width, height } - : undefined, + width !== undefined && height !== undefined ? { width, height } : undefined, }; } catch (err) { const message = err instanceof Error ? err.message : String(err); diff --git a/packages/flint-js/tests/validate.test.ts b/packages/flint-js/tests/validate.test.ts index c580fe34..1b761523 100644 --- a/packages/flint-js/tests/validate.test.ts +++ b/packages/flint-js/tests/validate.test.ts @@ -34,10 +34,7 @@ function withSpec(chart_spec: Partial): ChartA describe('validateChart', () => { it.each(VALIDATION_BACKENDS)('accepts a valid bar chart for %s', (backend) => { const result = validateChart(barChart, backend); - expect(result.backend).toBe(backend); - expect(result.chartType).toBe('Bar Chart'); - expect(result.valid).toBe(true); - expect(result.errors).toEqual([]); + expect(result).toMatchObject({ backend, chartType: 'Bar Chart', valid: true, errors: [] }); }); it('reports the computed layout size', () => { @@ -54,6 +51,12 @@ describe('validateChart', () => { expect(result.errors[0].message).toMatch(/Unknown chart type/); }); + it('flags an unknown backend as invalid without throwing', () => { + const result = validateChart(barChart, 'excel' as any); + expect(result.valid).toBe(false); + expect(result.errors[0].message).toMatch(/unknown backend: excel/); + }); + it('flags a nonexistent field', () => { const result = validateChart( withSpec({ encodings: { x: { field: 'missing' }, y: { field: 'revenue' } } }), @@ -74,24 +77,31 @@ describe('validateChart', () => { expect(result.errors[0].message).toContain('encodings.banana is not supported by Bar Chart for echarts'); }); - it('flags a canvas that exceeds the cap', () => { + it('flags a canvas that exceeds the default cap', () => { const result = validateChart(withSpec({ canvasSize: { width: 5000, height: 300 } }), 'vegalite'); expect(result.valid).toBe(false); expect(result.errors[0].message).toContain('maximum dimension of 4000px'); + }); - const relaxed = validateChart(withSpec({ canvasSize: { width: 5000, height: 300 } }), 'vegalite', { + it('honours a caller-supplied canvas cap', () => { + const result = validateChart(withSpec({ canvasSize: { width: 5000, height: 300 } }), 'vegalite', { maxCanvasDim: 8000, }); - expect(relaxed.valid).toBe(true); + expect(result.valid).toBe(true); }); - it('rejects malformed input without throwing', () => { - for (const input of [null, undefined, 42, {}, { data: {} }, { data: { url: 'x.csv' } }]) { - const result = validateChart(input as any, 'vegalite'); - expect(result.valid).toBe(false); - expect(result.chartType).toBe('(unknown)'); - expect(result.errors).toHaveLength(1); - } + it.each([ + ['null', null], + ['undefined', undefined], + ['a number', 42], + ['an empty object', {}], + ['data without values', { data: {} }], + ['data with a url only', { data: { url: 'x.csv' } }], + ['non-array values', { data: { values: 'nope' } }], + ])('rejects %s without throwing', (_label, input) => { + const result = validateChart(input as any, 'vegalite'); + expect(result).toMatchObject({ valid: false, chartType: '(unknown)' }); + expect(result.errors).toHaveLength(1); }); it('surfaces unregistered semantic_types as warnings, not errors', () => { @@ -100,24 +110,29 @@ describe('validateChart', () => { 'vegalite', ); expect(result.valid).toBe(true); - const warning = result.warnings.find((w) => w.code === 'unknown_semantic_type'); - expect(warning?.severity).toBe('warning'); - expect(warning?.field).toBe('revenue'); - expect(warning?.message).toContain('"Dollarz"'); + expect(result.warnings).toContainEqual( + expect.objectContaining({ severity: 'warning', code: 'unknown_semantic_type', field: 'revenue' }), + ); }); }); describe('validateChartInput', () => { - it('throws on the first problem', () => { + it('rejects encodings that bind no channel', () => { expect(() => validateChartInput(withSpec({ encodings: {} }), 'vegalite')).toThrow( /must bind at least one channel/, ); - expect(() => - validateChartInput({ ...barChart, data: { values: [] } }, 'vegalite'), - ).toThrow(/at least one row/); - expect(() => - validateChartInput(barChart, 'vegalite', { maxDataRows: 2 }), - ).toThrow(/exceeding the limit of 2/); + }); + + it('rejects empty data', () => { + expect(() => validateChartInput({ ...barChart, data: { values: [] } }, 'vegalite')).toThrow( + /at least one row/, + ); + }); + + it('honours a caller-supplied row cap', () => { + expect(() => validateChartInput(barChart, 'vegalite', { maxDataRows: 2 })).toThrow( + /exceeding the limit of 2/, + ); }); it('requires x and y for cartesian templates', () => { @@ -126,27 +141,33 @@ describe('validateChartInput', () => { ).toThrow(/encodings\.y is required for Bar Chart/); }); - it('checks fields without a backend', () => { - expect(() => - validateChartInput(withSpec({ encodings: { x: 'nope', y: 'revenue' } })), - ).toThrow(/"nope" does not exist/); + it('checks field existence even without a backend', () => { + expect(() => validateChartInput(withSpec({ encodings: { x: 'nope', y: 'revenue' } }))).toThrow( + /"nope" does not exist/, + ); expect(() => validateChartInput(barChart)).not.toThrow(); }); + + it('skips template checks for a chart type the backend does not know', () => { + expect(() => + validateChartInput(withSpec({ chartType: 'Not A Real Chart' }), 'vegalite'), + ).not.toThrow(); + }); }); describe('validateSemanticTypes', () => { - it('returns only unregistered labels', () => { - expect( - validateSemanticTypes({ - a: 'Amount', - b: { semanticType: 'Percentage' }, - c: 'NotAType', - d: { semanticType: '' }, - }), - ).toMatchObject([ - { severity: 'warning', code: 'unknown_semantic_type', field: 'c' }, - { severity: 'warning', code: 'unknown_semantic_type', field: 'd' }, - ]); + it('returns one warning per unregistered label', () => { + const warnings = validateSemanticTypes({ + a: 'Amount', + b: { semanticType: 'Percentage' }, + c: 'NotAType', + d: { semanticType: '' }, + }); + expect(warnings.map((w) => w.field)).toEqual(['c', 'd']); + expect(warnings.every((w) => w.code === 'unknown_semantic_type')).toBe(true); + }); + + it('returns nothing for missing semantic_types', () => { expect(validateSemanticTypes(undefined)).toEqual([]); }); }); @@ -158,11 +179,16 @@ describe('assembleForBackend', () => { expect(typeof width).toBe('number'); expect(typeof height).toBe('number'); expect(Object.keys(spec).some((k) => k.startsWith('_'))).toBe(true); - stripPrivateKeys(spec); - expect(Object.keys(spec).some((k) => k.startsWith('_'))).toBe(false); }); it('rejects an unknown backend', () => { expect(() => assembleForBackend('excel' as any, barChart)).toThrow(/unknown backend/); }); }); + +describe('stripPrivateKeys', () => { + it('removes only top-level underscore keys', () => { + const spec = stripPrivateKeys({ _warnings: [], width: 1, nested: { _keep: true } }); + expect(spec).toEqual({ width: 1, nested: { _keep: true } }); + }); +}); diff --git a/packages/flint-mcp/src/render/assemble.ts b/packages/flint-mcp/src/render/assemble.ts index 7572f3fe..df607286 100644 --- a/packages/flint-mcp/src/render/assemble.ts +++ b/packages/flint-mcp/src/render/assemble.ts @@ -25,7 +25,7 @@ export const MAX_DATA_ROWS = DEFAULT_MAX_DATA_ROWS; /** Maximum canvas dimension in pixels the host will honor (DoS guard). */ export const MAX_CANVAS_DIM = DEFAULT_MAX_CANVAS_DIM; -export const CAPS: ValidateChartOptions = { +export const INPUT_CAPS: ValidateChartOptions = { maxDataRows: MAX_DATA_ROWS, maxCanvasDim: MAX_CANVAS_DIM, }; @@ -39,7 +39,7 @@ export function validateInput( input: ChartAssemblyInput, options: DataSourceOptions = {}, ): void { - validateChartInput(resolveInput(input, options), undefined, CAPS); + validateChartInput(resolveInput(input, options), undefined, INPUT_CAPS); } /** Resolve `data.url` to inline rows so the core validator can see them. Throws on unreadable references. */ @@ -64,5 +64,5 @@ export function assembleForBackend( input: ChartAssemblyInput, options: DataSourceOptions = {}, ): AssembleResult { - return coreAssembleForBackend(backend, resolveInput(input, options), CAPS); + return coreAssembleForBackend(backend, resolveInput(input, options), INPUT_CAPS); } diff --git a/packages/flint-mcp/src/tools/validate.ts b/packages/flint-mcp/src/tools/validate.ts index 67b92b6b..a475d4f3 100644 --- a/packages/flint-mcp/src/tools/validate.ts +++ b/packages/flint-mcp/src/tools/validate.ts @@ -6,7 +6,7 @@ import { type ChartAssemblyInput, type ValidateResult, } from 'flint-chart'; -import { CAPS, resolveInput } from '../render/assemble.js'; +import { INPUT_CAPS, resolveInput } from '../render/assemble.js'; import type { DataSourceOptions } from '../render/data-source.js'; import type { RenderBackend } from '../render/types.js'; @@ -35,5 +35,5 @@ export function validateChart( errors: [{ severity: 'error', code: 'assembly_failed', message }], }; } - return coreValidateChart(resolvedInput, backend, CAPS); + return coreValidateChart(resolvedInput, backend, INPUT_CAPS); } From 300bf809a45ec985705c2315709ba073db7e3b1a Mon Sep 17 00:00:00 2001 From: taoche Date: Fri, 4 Sep 2026 17:01:38 +0800 Subject: [PATCH 4/4] fix(validate): check row shape and values/url conflict in core Hosts that bypass the MCP data-source layer previously got a misleading "field does not exist" (or a TypeError) for non-object rows, and silently accepted data carrying both values and url. Both checks now live in validateChartInput, matching what resolveDataSource already enforces. Co-Authored-By: Claude Code --- packages/flint-js/src/validate/index.ts | 10 +++++++++- packages/flint-js/tests/validate.test.ts | 12 ++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/flint-js/src/validate/index.ts b/packages/flint-js/src/validate/index.ts index ba6b584e..f5c14910 100644 --- a/packages/flint-js/src/validate/index.ts +++ b/packages/flint-js/src/validate/index.ts @@ -100,7 +100,7 @@ const TEMPLATE_LOOKUP: Record< /** * Validate the shape of a {@link ChartAssemblyInput} before it reaches an - * assembler: data presence and caps, `chartType`, encodings against the + * assembler: data presence, row shape and caps, `chartType`, encodings against the * backend's template (channel support, required channels, field existence), * and canvas caps. Throws on the first problem found. When `backend` is given * and the chart type is unknown to that backend, encoding checks are skipped @@ -135,6 +135,9 @@ function validateData(data: unknown, maxDataRows: number): Record maxDataRows) { throw new Error( `input.data.values has ${rows.length} rows, exceeding the limit of ${maxDataRows}`, @@ -143,6 +146,11 @@ function validateData(data: unknown, maxDataRows: number): Record { ); }); + it('rejects rows that are not objects', () => { + expect(() => + validateChartInput({ ...barChart, data: { values: [{ region: 'a', revenue: 1 }, 2] } }, 'vegalite'), + ).toThrow(/data row 2 must be an object/); + }); + + it('rejects data that provides both values and url', () => { + expect(() => + validateChartInput({ ...barChart, data: { values: barChart.data.values, url: 'x.json' } } as any, 'vegalite'), + ).toThrow(/either values or url, not both/); + }); + it('honours a caller-supplied row cap', () => { expect(() => validateChartInput(barChart, 'vegalite', { maxDataRows: 2 })).toThrow( /exceeding the limit of 2/,