diff --git a/docs/IMPLEMENTATION-PLAN.md b/docs/IMPLEMENTATION-PLAN.md index 0baffaf..07fb8ac 100644 --- a/docs/IMPLEMENTATION-PLAN.md +++ b/docs/IMPLEMENTATION-PLAN.md @@ -74,6 +74,10 @@ This is the at-a-glance list; keep it in sync with them. **Next (flagged for build):** +- **Multi-view data model** ([`multi-view-data-model-scope.md`](exploration/multi-view-data-model-scope.md)) — + durable composition support across the data-facing features. M1 (Vega-Lite-fidelity + reference classifier, `core/spec-data`) is done; M2–M4 extend the editor data context, + Extract, and the data inspector to be view-scoped. - **Chart Builder · 3B starter examples** ([`chart-builder-enhancement-scope.md`](exploration/chart-builder-enhancement-scope.md) §3) — a small set of curated starters, one per covered FT intent. Reshaped by 3C: a builder-openable starter must reference a dataset, so it ships paired sample datasets (or is diff --git a/docs/architecture/07-naming-and-relationships.md b/docs/architecture/07-naming-and-relationships.md index 0a5c128..67a0769 100644 --- a/docs/architecture/07-naming-and-relationships.md +++ b/docs/architecture/07-naming-and-relationships.md @@ -135,70 +135,67 @@ without waiting for a publish. Recomputation runs only on a _valid_ spec — auto-save is debounced and parse-gated (spec §03B) — so a transiently-invalid draft never disturbs the links. -### 3.1 Extracting referenced names from a spec (pure — `src/core/spec-refs.ts`) +### 3.1 What counts as a reference, and extracting them (pure — `src/core/spec-data.ts`, `spec-refs.ts`) -A Vega-Lite spec can reference named data in several places: the top-level -`data`, per-layer `data`, `data` inside `spec`/`facet`/`hconcat`/`vconcat`, and a -lookup transform's `from.data`. A spec may also define its OWN inline datasets via -a top-level `datasets` map — those are self-defined, not library references. -Rather than enumerate Vega-Lite's grammar, we walk the spec recursively and -collect every `{ data: { name } }` — but **prune two keys**: never recurse into a -`data` object's payload (its `values`/rows) or the top-level `datasets` map, -because those hold user data, not nested specs. Without the prune, a data _row_ -carrying a field literally named `data: { name: "x" }` is misread as a reference. -This is pure, deterministic, and the most heavily unit-tested function here. +A library reference is exactly Vega-Lite **named data**: a `data` block with a +string `name` and no `values`, `url`, or generator key (`sequence`/`sphere`/ +`graticule`), whose name the spec does not define for itself via a top-level +`datasets` map. A `name` riding on inline `values` or a `url` is Vega-Lite's +runtime-rebind label — not a dependency — and self-defined `datasets` names +resolve natively; both are left untouched. This mirrors Vega-Lite's own +`isNamedData`, so Astrolabe **extends** Vega-Lite rather than diverging: every +native data form keeps working, and only true references are tracked and resolved. + +That classification lives in **`core/spec-data`** (`classifyData`, +`libraryRefName`) — the single predicate that reference extraction, rename +(`spec-refs`), and render-time resolution (`rendering`) all route through, so they +cannot disagree on what is a dependency. ```ts -// src/core/spec-refs.ts +// src/core/spec-data.ts — the shared classifier (mirrors vega-lite/src/data.ts) -type Json = unknown; +/** The library name a `data` block references, or null for native VL data / self-defined names. */ +export function libraryRefName(data: unknown, selfDefined: ReadonlySet): string | null { + if (classifyData(data) !== 'named') return null; // url | values | generator → not a reference + const { name } = data as { name: string }; + return selfDefined.has(name) ? null : name; +} +``` + +References appear in several places — top-level `data`, per-layer `data`, `data` +inside `spec`/`facet`/`hconcat`/`vconcat`, and a lookup transform's `from.data`. +Rather than enumerate the grammar, each pass walks the spec recursively but +**prunes two keys**: a `data` object's payload (its `values`/rows) and the +top-level `datasets` map hold user data, not nested specs. Without the prune, a +data _row_ carrying a field literally named `data: { name: "x" }` is misread as a +reference. + +```ts +// src/core/spec-refs.ts — the recursive walk; classification routes through spec-data -/** Collects every library dataset name referenced by `{ data: { name } }`, excluding self-defined ones. */ export function extractDatasetRefs(spec: Json): string[] { - const root = typeof spec === 'string' ? safeParse(spec) : spec; - const selfDefined = selfDefinedNames(root); // names from the spec's own top-level `datasets` + const root = typeof spec === 'string' ? safeParse(spec) : spec; // unparseable → no refs + const selfDefined = selfDefinedNames(root); const names = new Set(); - const walk = (node: Json): void => { - if (Array.isArray(node)) { - for (const item of node) walk(item); - return; - } + if (Array.isArray(node)) return void node.forEach(walk); if (node && typeof node === 'object') { const obj = node as Record; - const data = obj.data as Record | undefined; - if (data && typeof data === 'object' && typeof data.name === 'string') { - if (!selfDefined.has(data.name)) names.add(data.name); - } - // Prune: a `data` payload and the `datasets` map hold user data, not refs. + const refName = libraryRefName(obj.data, selfDefined); + if (refName !== null) names.add(refName); for (const key of Object.keys(obj)) { - if (key === 'data' || key === 'datasets') continue; + if (key === 'data' || key === 'datasets') continue; // prune user-data payloads walk(obj[key]); } } }; - walk(root); return [...names]; } - -function safeParse(s: string): Json { - try { - return JSON.parse(s); - } catch { - return null; // an unparseable draft simply has no resolvable refs - } -} ``` -```ts -// src/core/spec-refs.ts — thin wrapper, run on every draft change and on publish - -/** The list stored on snippet.datasetRefs. Sorted + de-duped for stable diffs. */ -export function recomputeDatasetRefs(spec: Json): string[] { - return extractDatasetRefs(spec).sort(); -} -``` +`recomputeDatasetRefs` is the thin sorted/de-duped wrapper stored on +`snippet.datasetRefs`, run on every draft change and on publish. > A `spec` may be an object or a string (see the Data Model). Normalize once, > at the boundary, so the recursive walk never has to care. @@ -207,17 +204,19 @@ export function recomputeDatasetRefs(spec: Json): string[] { - Treat `extractDatasetRefs` as the single source of truth for "what does this spec reference". The reverse-lookup and rename paths both depend on it - agreeing with what the renderer actually resolves. + agreeing with what the renderer actually resolves — which holds because all of + them classify through the same `core/spec-data` predicate. - Recompute and store `datasetRefs` on **every draft change and on publish** — but only through the parse-gated, debounced auto-save (`commitDraft`) and the programmatic extract/revert rewrites, never on raw keystrokes. That keeps the links in step with the edited draft while never recomputing from a transiently-invalid spec. -- Prune the **same two keys** (`data`, `datasets`) in all three ref walks — - extraction here, `renameDatasetInSpec`, and the renderer's `resolveDatasetRefs` - (`src/core/rendering.ts`). They must agree on what counts as a reference; if one - descends into data payloads and another doesn't, extraction and rendering - disagree and a row field named `data` either gets counted, rewritten, or throws +- Keep the three ref walks in lockstep — extraction here, `renameDatasetInSpec`, + and the renderer's `resolveDatasetRefs` (`src/core/rendering.ts`). They agree + because they share both halves: the `libraryRefName` classifier (what is a + reference) and the prune of the **same two keys** (`data`, `datasets`). If one + classified differently, or descended into data payloads while another didn't, a + row field named `data` would get counted, rewritten, or throw `DatasetNotFoundError`. **Don't** diff --git a/docs/exploration/multi-view-data-model-scope.md b/docs/exploration/multi-view-data-model-scope.md new file mode 100644 index 0000000..584350a --- /dev/null +++ b/docs/exploration/multi-view-data-model-scope.md @@ -0,0 +1,84 @@ +# Multi-view data model — scope & plan + +Astrolabe authors arbitrary Vega-Lite, including **composed** specs (`layer`, +`hconcat`/`vconcat`/`concat`, `facet`, `repeat`). Most of the spec-structure +machinery already handles composition; the data-facing features carried +single-view assumptions. This memo records the assessment, the data-model +contract that anchors the work, and the milestone plan to make multi-view support +durable. The guiding constraint: **extend Vega-Lite, never break it** — every +native data form must keep working. + +## The data-model contract + +A dataset reference is exactly Vega-Lite **named data**: a `data` block with a +string `name` and no `values`/`url`/generator key, whose name the spec does not +self-define via top-level `datasets`. This mirrors Vega-Lite's `isNamedData` +(`reference/vega-lite/src/data.ts`). The classification is owned by +`core/spec-data` (`classifyData`, `libraryRefName`); reference extraction +(`spec-refs`), rename (`spec-refs`), and render-time resolution (`rendering`) all +route through it. See `docs/architecture/07` §3.1. + +Library references resolve to inline data before embedding +(`core/rendering` → `prepareSpecForRender`); a self-defined `datasets` name is +left for Vega-Lite to resolve natively. + +## Assessment: already multi-view vs. single-view assumptions + +**Already composition-aware** (recurse all view operators): ref extraction/rename +(`spec-refs`), reference resolution + fit-mode (`rendering`), structural wrap/ +unwrap/add-view (`spec-transforms`, `spec-cursor`, `spec-insert`), derived-field +collection (`spec-fields`), config baking (`spec-config`), standalone export +(`chart-export`, reuses `prepareSpecForRender`). + +**Single-view assumptions** (the work): + +- **Editor data context** (`app/services/active-dataset`) resolves _one_ dataset + for the whole draft (first ref, or first inline data), with no notion of which + view the cursor sits in. Completion/hover/inlay (`spec-dataset-hints`) and the + facet/repeat field defaults (`spec-transform-actions`) therefore offer the wrong + view's columns in a composition whose views bind different datasets. +- **Extract inline data** (`app/stores/ExtractStore`) lifts only the top-level + `data` block. +- **Data inspector** (`core/result-data`, `DataInspector`) surfaces one input + + one resolved table; a composition produces several `source_`/`data_`. +- **`deriveSnippetName`** (`core/snippet`) reads top-level `mark`/`encoding` only, + so a composed spec falls back to the default name (graceful, not a bug). +- **Chart builder** is single-view by design; its strict round-trip hydration + returns `null` for composed specs, so they stay Monaco-only (correct). + +## Vega-Lite fidelity clashes + +1. **Named inline/url data misread as a reference** — classifying on "has a + string `name`" alone caught named-inline (`{ name, values }`) and named-url, + breaking valid specs (spurious `DatasetNotFoundError`, or clobbered inline + values). _Resolved_ by the `core/spec-data` classifier (M1). +2. **Shadowing** — a library dataset whose name equals a self-defined `datasets` + key is silently ignored (self-defined wins). Documented precedence; candidate + for a user-facing note, no code change required. +3. **Case-rule split** — library matching is case-insensitive (`naming.ts`); + self-defined exclusion and Vega-Lite's own named-data lookup are case-sensitive. + These are distinct namespaces, so the split is defensible; minor. +4. **Runtime-injected named data** — Vega-Lite allows binding `{ name }` at + runtime; Astrolabe always pre-resolves, so an imported spec relying on runtime + injection won't render. Out of scope. + +## Milestone plan + +- **M1 — data-model foundation** ✅ — `core/spec-data` classifier mirroring + `isNamedData`; `spec-refs` + `rendering` routed through it. Closes clash 1. +- **M2 — view-scoped editor context** — pure `dataContextAtPath(spec, path)`: + climb the cursor's JSON path to the nearest enclosing `data` (honoring + Vega-Lite's parent→child data inheritance), classify it, and collect + ancestor-chain derived fields. Rework `active-dataset` to be cursor-scoped and + thread the offset through the three Monaco providers and the facet/repeat + defaults. Resolve columns for every form: library ref, inline `values`, + named-inline, self-defined `datasets`, url (no static rows), generator (none). +- **M3 — view-scoped extract** — seed Extract from the focused view's inline data + (reusing the cursor-scope machinery) and rewrite that view's `data`. +- **M4 — multi-view inspection** — group the dataflow's datasets into per-view + input/resolved pairs (`result-data`) and add a view selector to `DataInspector` + (new interactive widget → `/council`). + +Delivery is incremental, one milestone per commit, verified against real behavior. +The consolidated data-model contract write-up into `docs/architecture` (05/08) +lands once the shape is final. diff --git a/src/core/rendering.test.ts b/src/core/rendering.test.ts index 56b8884..86b9501 100644 --- a/src/core/rendering.test.ts +++ b/src/core/rendering.test.ts @@ -203,6 +203,26 @@ describe('prepareSpecForRender — dataset resolution (spec §04 Rendering Contr expect(out.data).toEqual({ name: 'local' }); }); + test('native Vega-Lite data with a name label is left untouched (not resolved, never throws)', () => { + // A name on inline/url data, or a generator, is native Vega-Lite — even when + // the name is absent from the library. Resolution must not touch or reject it. + const inline = { data: { name: 'pts', values: [{ a: 1 }] }, mark: 'point' }; + expect(prepareSpecForRender(inline, { datasets }).data).toEqual({ + name: 'pts', + values: [{ a: 1 }], + }); + const url = { data: { name: 'remote', url: 'https://x/y.csv' } }; + expect(prepareSpecForRender(url, { datasets }).data).toEqual({ + name: 'remote', + url: 'https://x/y.csv', + }); + const gen = { data: { name: 'seq', sequence: { start: 0, stop: 5 } } }; + expect(prepareSpecForRender(gen, { datasets }).data).toEqual({ + name: 'seq', + sequence: { start: 0, stop: 5 }, + }); + }); + test('resolution runs before fit-mode: a ref + fit mode produce both transforms', () => { const spec = { data: { name: 'JsonDs' }, mark: 'bar' }; const out = prepareSpecForRender(spec, { datasets, fitMode: 'full' }) as unknown as { diff --git a/src/core/rendering.ts b/src/core/rendering.ts index a46c2f0..b486862 100644 --- a/src/core/rendering.ts +++ b/src/core/rendering.ts @@ -10,13 +10,15 @@ * 1. Dataset reference resolution — implemented in M3 (see below). * 2. Fit-mode sizing — implemented in M2. * - * Step 1 (spec §04 → Rendering Contract): every named-data reference + * Step 1 (spec §04 → Rendering Contract): every library reference * (`{ data: { name } }`) is replaced in-place with the referenced library - * dataset's actual contents, shaped by source and format. A name the spec defines - * for itself via a top-level `datasets` object is left untouched (Vega-Lite - * resolves it natively); an unknown library name throws `DatasetNotFoundError`. - * Resolution recurses into the same nested sub-specs as fit-mode, runs before - * sizing, and operates only on the copy. + * dataset's actual contents, shaped by source and format. What counts as a library + * reference is decided by `core/spec-data` (named data, not self-defined), so a + * `name` riding on inline `values`/`url`/a generator is native Vega-Lite data and + * is left untouched; a name the spec defines for itself via a top-level `datasets` + * object is left untouched too (Vega-Lite resolves it natively); an unknown library + * name throws `DatasetNotFoundError`. Resolution recurses into the same nested + * sub-specs as fit-mode, runs before sizing, and operates only on the copy. * * The copy-not-mutate invariant and the call site the renderer depends on are * fixed. @@ -24,6 +26,7 @@ import type { DataFormat } from './format-detection'; import type { DataSource } from './dataset'; +import { libraryRefName, selfDefinedNames } from './spec-data'; /** Preview sizing modes (spec §04 → Fit / Sizing Modes). `default` = Original. */ export type FitMode = 'default' | 'width' | 'height' | 'full'; @@ -113,16 +116,6 @@ function applyFitMode(node: unknown, mode: FitMode): void { if (isSpecNode(node.spec)) applyFitMode(node.spec, mode); } -/** The set of dataset names a spec defines for itself via top-level `datasets`. */ -function selfDefinedDatasetNames(spec: unknown): Set { - const names = new Set(); - if (isSpecNode(spec)) { - const datasets = spec.datasets; - if (isSpecNode(datasets)) for (const key of Object.keys(datasets)) names.add(key); - } - return names; -} - /** * Build the replacement `data` object for one resolved reference (spec §04 → * Rendering Contract, step 1). `rest` is the reference's other keys (e.g. a @@ -157,13 +150,16 @@ function resolvedData(dataset: ResolvableDataset, rest: SpecNode): SpecNode { } /** - * Replace every named-data reference in `node` with its library dataset's - * contents, recursing through the spec (arrays and objects) so refs anywhere are - * resolved — matching `extractDatasetRefs`, including pruning the `data` and - * top-level `datasets` payload keys so resolution never descends into user data - * rows. A self-defined name is left untouched; an unknown library name throws - * `DatasetNotFoundError`. Matching is case-insensitive, mirroring naming.ts. - * Mutates in place; the caller already works on a copy. + * Replace every library reference in `node` with its dataset's contents, recursing + * through the spec (arrays and objects) so refs anywhere are resolved — matching + * `extractDatasetRefs`, including pruning the `data` and top-level `datasets` + * payload keys so resolution never descends into user data rows. What counts as a + * library reference is decided by the shared `libraryRefName` (core/spec-data): + * named data the spec does not define for itself. A `name` riding on inline + * `values`, a `url`, or a generator is native Vega-Lite data and is left untouched; + * an unknown library name throws `DatasetNotFoundError`. Matching is + * case-insensitive, mirroring naming.ts. Mutates in place; the caller already works + * on a copy. */ function resolveDatasetRefs( node: unknown, @@ -176,15 +172,12 @@ function resolveDatasetRefs( } if (!isSpecNode(node)) return; - const data = node.data; - if (isSpecNode(data) && typeof data.name === 'string') { - const name = data.name; - if (!selfDefined.has(name)) { - const dataset = byName.get(name.toLowerCase()); - if (!dataset) throw new DatasetNotFoundError(name); - const { name: _drop, ...rest } = data; - node.data = resolvedData(dataset, rest); - } + const refName = libraryRefName(node.data, selfDefined); + if (refName !== null) { + const dataset = byName.get(refName.toLowerCase()); + if (!dataset) throw new DatasetNotFoundError(refName); + const { name: _drop, ...rest } = node.data as SpecNode; + node.data = resolvedData(dataset, rest); } // Recurse into every key except the two that hold data payloads (`data` — @@ -229,7 +222,7 @@ export function prepareSpecForRender(spec: T, options: PrepareOptions = {}): const datasets = options.datasets ?? []; const byName = new Map(); for (const d of datasets) byName.set(d.name.toLowerCase(), d); - resolveDatasetRefs(copy, byName, selfDefinedDatasetNames(copy)); + resolveDatasetRefs(copy, byName, selfDefinedNames(copy)); // 2. Fit-mode sizing. applyFitMode(copy, options.fitMode ?? 'default'); diff --git a/src/core/spec-data.test.ts b/src/core/spec-data.test.ts new file mode 100644 index 0000000..8cdd431 --- /dev/null +++ b/src/core/spec-data.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, test } from 'vitest'; +import { classifyData, libraryRefName, selfDefinedNames } from './spec-data'; + +describe('classifyData', () => { + test('classifies the four Vega-Lite data shapes', () => { + expect(classifyData({ url: 'x.csv' })).toBe('url'); + expect(classifyData({ values: [{ a: 1 }] })).toBe('inline'); + expect(classifyData({ sequence: { start: 0, stop: 5 } })).toBe('generator'); + expect(classifyData({ sphere: true })).toBe('generator'); + expect(classifyData({ graticule: true })).toBe('generator'); + expect(classifyData({ name: 'Sales' })).toBe('named'); + }); + + test('url / values / generator win over a bare name (Vega-Lite precedence)', () => { + // A name on these shapes is a runtime-rebind label, not a named reference. + expect(classifyData({ name: 'pts', values: [{ a: 1 }] })).toBe('inline'); + expect(classifyData({ name: 'remote', url: 'x.csv' })).toBe('url'); + expect(classifyData({ name: 'seq', sequence: { start: 0, stop: 5 } })).toBe('generator'); + }); + + test('returns null for non-data values', () => { + expect(classifyData(null)).toBeNull(); + expect(classifyData(undefined)).toBeNull(); + expect(classifyData('string')).toBeNull(); + expect(classifyData([])).toBeNull(); + expect(classifyData({})).toBeNull(); + expect(classifyData({ name: 123 })).toBeNull(); // a non-string name is not named data + }); +}); + +describe('selfDefinedNames', () => { + test('collects top-level datasets keys', () => { + expect([...selfDefinedNames({ datasets: { a: [], b: [] } })].sort()).toEqual(['a', 'b']); + }); + + test('is empty when there is no datasets map', () => { + expect(selfDefinedNames({ data: { name: 'x' } }).size).toBe(0); + expect(selfDefinedNames(null).size).toBe(0); + }); +}); + +describe('libraryRefName', () => { + const none = new Set(); + + test('returns the name of a true named reference', () => { + expect(libraryRefName({ name: 'Sales' }, none)).toBe('Sales'); + }); + + test('returns null for inline/url/generator data carrying a name', () => { + expect(libraryRefName({ name: 'pts', values: [{ a: 1 }] }, none)).toBeNull(); + expect(libraryRefName({ name: 'remote', url: 'x.csv' }, none)).toBeNull(); + expect(libraryRefName({ name: 'seq', sequence: { start: 0, stop: 5 } }, none)).toBeNull(); + }); + + test('returns null for a self-defined name', () => { + expect(libraryRefName({ name: 'local' }, new Set(['local']))).toBeNull(); + }); + + test('returns null for a non-data value', () => { + expect(libraryRefName(null, none)).toBeNull(); + expect(libraryRefName({ values: [{ a: 1 }] }, none)).toBeNull(); + }); +}); diff --git a/src/core/spec-data.ts b/src/core/spec-data.ts new file mode 100644 index 0000000..c931c06 --- /dev/null +++ b/src/core/spec-data.ts @@ -0,0 +1,74 @@ +/** + * The Vega-Lite data model — the single place that decides what a `data` block + * *means* (docs/architecture/05 → Rendering Contract; 07 → Relationships). Pure + * core: no browser APIs, no React. + * + * A Vega-Lite `data` block is exactly one of four shapes (mirrored from + * vega-lite/src/data.ts — `isUrlData`/`isInlineData`/`isNamedData`/`isGenerator`): + * + * - **url** — has `url` + * - **inline** — has `values` (and MAY also carry a `name` label) + * - **generator** — has `sequence` | `sphere` | `graticule` + * - **named** — has a string `name`, and NONE of the above + * + * Astrolabe binds its library datasets through *named* data: `{ data: { name } }` + * resolves to the library dataset's rows at render time (core/rendering). The + * fidelity rule, taken verbatim from Vega-Lite, is that a `name` ALONE does not + * make a reference — a `name` riding on inline `values`, a `url`, or a generator + * is a label Vega-Lite uses to rebind data at runtime, and Astrolabe must leave it + * untouched. Classifying by this rule (not by "has a string name") is what keeps + * Astrolabe a strict *extension* of Vega-Lite rather than a divergence: every + * native data form keeps working, and only true named references are resolved. + * + * Reference detection (`extractDatasetRefs`/rename in core/spec-refs) and reference + * resolution (core/rendering) both run through `libraryRefName` here, so the two + * can never disagree on what counts as a library dependency. + */ + +import { isJsonObject } from './spec-config'; + +/** The four Vega-Lite data shapes; `null` when the value is not a data block. */ +export type DataKind = 'url' | 'inline' | 'generator' | 'named'; + +/** The keys that mark a Vega-Lite data *generator* (sequence/sphere/graticule). */ +const GENERATOR_KEYS = ['sequence', 'sphere', 'graticule'] as const; + +/** + * Classify a Vega-Lite `data` value, matching Vega-Lite's own precedence: `url` + * and `values` and generator keys win over a bare `name`, so a named-inline or + * named-url block is inline/url data — not a named reference. Returns `null` for a + * non-object, `data: null`, or an empty/unrecognized block. + */ +export function classifyData(data: unknown): DataKind | null { + if (!isJsonObject(data)) return null; + if ('url' in data) return 'url'; + if ('values' in data) return 'inline'; + if (GENERATOR_KEYS.some((key) => key in data)) return 'generator'; + if (typeof data.name === 'string') return 'named'; + return null; +} + +/** + * The set of dataset names a spec defines for itself via a top-level `datasets` + * object. These are resolved natively by Vega-Lite, so they are never library + * dependencies and are excluded from reference detection and resolution. + */ +export function selfDefinedNames(spec: unknown): Set { + const names = new Set(); + if (isJsonObject(spec) && isJsonObject(spec.datasets)) { + for (const key of Object.keys(spec.datasets)) names.add(key); + } + return names; +} + +/** + * The library dataset name a `data` block references, or `null` when it is not a + * library reference — i.e. it is named data (`classifyData` → `'named'`) whose + * name is not one the spec defines for itself. This is the single predicate behind + * both reference extraction and render-time resolution. + */ +export function libraryRefName(data: unknown, selfDefined: ReadonlySet): string | null { + if (classifyData(data) !== 'named') return null; + const { name } = data as { name: string }; + return selfDefined.has(name) ? null : name; +} diff --git a/src/core/spec-refs.test.ts b/src/core/spec-refs.test.ts index c4e89f1..703025f 100644 --- a/src/core/spec-refs.test.ts +++ b/src/core/spec-refs.test.ts @@ -33,11 +33,24 @@ describe('extractDatasetRefs', () => { expect(extractDatasetRefs('{ not valid json')).toEqual([]); }); - test('inline-data and url-data references (no name) are ignored', () => { + test('inline-data and url-data (no name) are not references', () => { expect(extractDatasetRefs({ data: { values: [{ a: 1 }] } })).toEqual([]); expect(extractDatasetRefs({ data: { url: 'http://x/y.csv' } })).toEqual([]); }); + test('a name on inline/url data is a Vega-Lite label, not a library reference', () => { + // `{ name, values }` is inline data and `{ name, url }` is url data — the name + // is Vega-Lite's runtime-rebind label, not a dependency (vega-lite/src/data.ts). + expect(extractDatasetRefs({ data: { name: 'pts', values: [{ a: 1 }] } })).toEqual([]); + expect(extractDatasetRefs({ data: { name: 'remote', url: 'http://x/y.csv' } })).toEqual([]); + }); + + test('generator data is not a reference', () => { + expect(extractDatasetRefs({ data: { sequence: { start: 0, stop: 10 } } })).toEqual([]); + expect(extractDatasetRefs({ data: { name: 'globe', sphere: true } })).toEqual([]); + expect(extractDatasetRefs({ data: { graticule: true } })).toEqual([]); + }); + test('excludes names the spec defines for itself via top-level datasets', () => { const spec = { datasets: { foo: [{ a: 1 }] }, @@ -128,14 +141,16 @@ describe('renameDatasetInSpec', () => { expect(Object.keys(out.datasets)).toEqual(['Old']); }); - test('does not rewrite a "data" field buried in inline data rows', () => { + test('does not rewrite a named-inline block or a "data" field buried in its rows', () => { + // `{ name, values }` is inline data — the name is a Vega-Lite label, not a + // library reference — so neither it nor the row payload is renamed. const spec = { data: { name: 'Old', values: [{ data: { name: 'Old' } }] }, mark: 'bar', }; const out = renameDatasetInSpec(spec, 'Old', 'New'); - expect(out.data.name).toBe('New'); // the real reference is renamed - expect(out.data.values[0].data.name).toBe('Old'); // the row payload is left alone + expect(out.data.name).toBe('Old'); + expect(out.data.values[0].data.name).toBe('Old'); }); test('renames a reference inside a lookup transform (from.data)', () => { diff --git a/src/core/spec-refs.ts b/src/core/spec-refs.ts index 8227af4..2756476 100644 --- a/src/core/spec-refs.ts +++ b/src/core/spec-refs.ts @@ -3,34 +3,35 @@ * (spec §09F → Cross-entity relationships; docs/architecture/07 §3.1, §6). * * Portable core: no browser APIs, no React, no store access. A Vega-Lite spec - * references named data through `{ "data": { "name": "MyDataset" } }`, which can - * appear at the top level, per-layer, inside `spec`/`facet`/concat children, or in - * a lookup transform's `from.data`. Rather than enumerate the grammar, we walk the - * spec recursively and collect every `{ data: { name } }` we find — the single - * source of truth for "what does this spec reference", which the renderer's - * resolution must agree with. + * references a library dataset through *named* data — `{ "data": { "name": "X" } }` + * — which can appear at the top level, per-layer, inside `spec`/`facet`/concat + * children, or in a lookup transform's `from.data`. Rather than enumerate the + * grammar, we walk the spec recursively and collect every library reference we + * find — the single source of truth for "what does this spec reference", which the + * renderer's resolution must agree with. + * + * What counts as a *library reference* is decided by `libraryRefName` + * (core/spec-data): named data (a string `name` with no `values`/`url`/generator) + * whose name the spec does not define for itself via a top-level `datasets` object. + * A `name` riding on inline `values` or a `url` is a Vega-Lite label, not a + * dependency, so it is left alone — keeping Astrolabe a strict extension of + * Vega-Lite. The renderer resolves through the same predicate, so the two cannot + * disagree. * * The walk recurses into every key EXCEPT two, which hold user data payloads - * rather than nested specs: a `data` object (its `name` is captured at the parent - * site; its `values`/`format` are payload, never a nested ref) and a top-level - * `datasets` map (the spec's own inline data). Pruning those is what keeps a data - * *row* that happens to carry a field literally named `data: { name: "x" }` from - * being misread as a library reference. The renderer's resolution prunes the same - * two keys so the two stay in lockstep. + * rather than nested specs: a `data` object (classified at the parent site; its + * `values`/`format` are payload, never a nested ref) and a top-level `datasets` + * map (the spec's own inline data). Pruning those is what keeps a data *row* that + * happens to carry a field literally named `data: { name: "x" }` from being misread + * as a library reference. The renderer's resolution prunes the same two keys. * * A spec may be stored as an **object** or as **JSON text** (see spec §09A); we * normalize once at the boundary (unparseable text → no refs / unchanged spec) so * the recursive walk never has to care. - * - * Refinement beyond the doc sketch: a spec may define its OWN inline named - * datasets via a top-level `datasets` object (e.g. - * `{ "datasets": { "foo": [...] }, "data": { "name": "foo" } }`). Names satisfied - * by the spec's own `datasets` are NOT library dependencies, so they are excluded - * from extraction and left untouched on rename — keeping extraction consistent - * with the renderer's resolution, which likewise must not treat self-defined - * names as library refs. */ +import { libraryRefName, selfDefinedNames } from './spec-data'; + type Json = unknown; /** Parse a string spec; an unparseable draft simply has no resolvable refs. */ @@ -42,22 +43,10 @@ function safeParse(s: string): Json { } } -/** The set of dataset names a spec defines for itself via top-level `datasets`. */ -function selfDefinedNames(spec: Json): Set { - const names = new Set(); - if (spec && typeof spec === 'object' && !Array.isArray(spec)) { - const datasets = (spec as Record).datasets; - if (datasets && typeof datasets === 'object' && !Array.isArray(datasets)) { - for (const key of Object.keys(datasets)) names.add(key); - } - } - return names; -} - /** - * Collects every **library** dataset name referenced by `{ data: { name } }` - * anywhere in the spec, excluding names the spec defines for itself via a - * top-level `datasets` object. Accepts an object or JSON text. + * Collects every **library** dataset name referenced anywhere in the spec + * (`libraryRefName`), excluding names the spec defines for itself via a top-level + * `datasets` object. Accepts an object or JSON text. */ export function extractDatasetRefs(spec: Json): string[] { const root = typeof spec === 'string' ? safeParse(spec) : spec; @@ -71,12 +60,10 @@ export function extractDatasetRefs(spec: Json): string[] { } if (node && typeof node === 'object') { const obj = node as Record; - const data = obj.data as Record | undefined; - if (data && typeof data === 'object' && typeof data.name === 'string') { - if (!selfDefined.has(data.name)) names.add(data.name); - } + const refName = libraryRefName(obj.data, selfDefined); + if (refName !== null) names.add(refName); // Recurse into every key except the two that hold data payloads (`data` — - // captured above; `datasets` — the spec's own inline data). Pruning them + // classified above; `datasets` — the spec's own inline data). Pruning them // keeps the walk out of user data rows, where a field named `data` would // otherwise be misread as a reference. Kept in step with rendering.ts. for (const key of Object.keys(obj)) { @@ -96,12 +83,14 @@ export function recomputeDatasetRefs(spec: Json): string[] { } /** - * Returns a copy of `spec` with every `data.name === oldName` replaced by - * `newName`, recursing through arrays and objects. A name that is a top-level - * `datasets` key (self-defined) is left untouched. The input's stored shape is - * preserved: a string spec is parsed, rewritten, and re-serialized as pretty JSON - * text; an object spec returns an object. The generic `` reflects this shape - * preservation. The input is never mutated. + * Returns a copy of `spec` with every library reference to `oldName` (per + * `libraryRefName`) renamed to `newName`, recursing through arrays and objects. + * Native Vega-Lite data that merely carries the name as a label (inline `values`, + * a `url`, a generator) and a self-defined top-level `datasets` key are left + * untouched. The input's stored shape is preserved: a string spec is parsed, + * rewritten, and re-serialized as pretty JSON text; an object spec returns an + * object. The generic `` reflects this shape preservation; the input is never + * mutated. */ export function renameDatasetInSpec(spec: T, oldName: string, newName: string): T { const isString = typeof spec === 'string'; @@ -116,15 +105,17 @@ export function renameDatasetInSpec(spec: T, oldName: string, newName: string if (node && typeof node === 'object') { const out: Record = {}; for (const [k, v] of Object.entries(node as Record)) { - // A `data` object is a reference site, not a container: rename a matching - // name and stop — never recurse into its payload. A `datasets` map is the - // spec's own inline data: leave it whole. Pruning both (mirroring - // extractDatasetRefs) keeps rename out of user data rows, where a field - // named `data` would otherwise be rewritten as if it were a reference. - if (k === 'data' && v && typeof v === 'object' && !Array.isArray(v)) { - const dv = v as Record; - out[k] = dv.name === oldName && !selfDefined.has(oldName) ? { ...dv, name: newName } : dv; - } else if (k === 'data' || k === 'datasets') { + // A `data` block is a reference site, not a container: rename it when it is + // a library reference to `oldName`, and never recurse into its payload. A + // `datasets` map is the spec's own inline data: leave it whole. Pruning + // both (mirroring extractDatasetRefs) keeps rename out of user data rows, + // where a field named `data` would otherwise be rewritten as a reference. + if (k === 'data') { + out[k] = + libraryRefName(v, selfDefined) === oldName + ? { ...(v as Record), name: newName } + : v; + } else if (k === 'datasets') { out[k] = v; } else { out[k] = rewrite(v);