diff --git a/docs/architecture/01-state-and-stores.md b/docs/architecture/01-state-and-stores.md index 4cf626c..4e1d8be 100644 --- a/docs/architecture/01-state-and-stores.md +++ b/docs/architecture/01-state-and-stores.md @@ -165,6 +165,20 @@ const active = useSnippetStore(selectActiveSnippet); > Rule: if you can compute it, do not store it. Add a new state field only for a > value that is _input_ the app receives, not output it derives. +### Editing buffers — the sanctioned duplication, and its sync rule + +A text field with debounced auto-save (the metadata panel's Name/Comment, the editor +buffer) legitimately mirrors a store fact into local component state: the local copy is +the user's in-progress text, the store holds the saved value. This duplication carries an +obligation the moment the store fact has **another writer** (publish's content-derived +renaming, import, any store-side mutation): the component must **adopt** a store change it +didn't make, or its debounced save will write the stale local copy back — silently undoing +the other writer. The pattern (see `SnippetLibrary`'s `SnippetMeta`): track the last store +value seen in a ref; when the store value changes, adopt it into local state **unless the +user has diverged** (local ≠ previous store value) — in-progress typing always wins. +Keying the component by entity id handles switching entities; this rule handles the same +entity changing underneath. + --- ## 3. Where State Lives: Central vs. Per-Feature Stores diff --git a/docs/architecture/07-naming-and-relationships.md b/docs/architecture/07-naming-and-relationships.md index 0fda734..0a5c128 100644 --- a/docs/architecture/07-naming-and-relationships.md +++ b/docs/architecture/07-naming-and-relationships.md @@ -464,7 +464,30 @@ user action — there is no separate coordinator module to call. --- -## 7. Where things live +## 7. Snippet name provenance — the naming hierarchy + +Snippet names (unlike dataset names) need no uniqueness; what they need is a rule for +**who may rewrite them**. Each snippet carries `nameSource` (spec §09A): `'user'` names +are frozen — set by an explicit rename (`SnippetStore.renameSnippet`) and never touched +by the app again; `'auto'` names are app-picked and keep tracking the spec. On publish, +an auto-named snippet is re-named from the now-published content in priority order: the +spec's `title` (string, line array, or `{ text }` forms), else a mark + encodings +description, else the existing name stands. The derivation dialect is deliberately the +same one `generateChartName` uses for builder output, so manually authored and +builder-built snippets read alike in the library. + +Flow: `core/snippet.ts` (`deriveSnippetName`, `isAutoNamed`, `isDefaultSnippetName`) → +`SnippetStore.publish` (the only rewrite site) / `renameSnippet` (the freeze site) → +`SnippetLibrary`'s metadata panel (which must adopt a publish rename — arch 01 §2, +editing buffers). Records predating `nameSource` have no provenance; `isAutoNamed` +treats them as user-named unless the name is **provably** app-picked — the timestamp +default shape, or identical to what `deriveSnippetName` returns for the record's own +published spec. The conservative default is deliberate: rewriting a chosen name is worse +than failing to track an auto one. + +--- + +## 8. Where things live | Concern | Location | Pure? | Tested | | ------------------------------------------------------------------------ | -------------------------------- | ------------------- | ----------- | @@ -474,6 +497,7 @@ user action — there is no separate coordinator module to call. | `snippetsReferencingDataset`, `datasetUsageCounts` (reverse-lookup scan) | `src/core/relationships.ts` | yes | unit | | `renameDatasetRefs` → updated count (rename propagation) | `src/app/stores/SnippetStore.ts` | no (mutates stores) | integration | | `dedupeIncomingNames` (datasets + custom themes) | `src/core/import-normalize.ts` | yes | unit | +| `deriveSnippetName`, `isAutoNamed` (snippet name provenance) | `src/core/snippet.ts` | yes | unit | The dividing line: anything that takes plain data and returns plain data is **core** and unit-tested in isolation; anything that reaches into a Zustand store diff --git a/docs/spec/02-snippet-library.md b/docs/spec/02-snippet-library.md index e3437a9..fe24c51 100644 --- a/docs/spec/02-snippet-library.md +++ b/docs/spec/02-snippet-library.md @@ -87,6 +87,7 @@ The library provides the lifecycle operations for snippets. An operation whose o New snippets get a sensible default name, and a tag field exists on each snippet for categorization, though tags are not a primary user surface. - A new snippet receives an auto-generated default name based on the current date and time, so it is uniquely identifiable until the user renames it (renaming happens in the metadata panel). +- Names follow a **provenance hierarchy**: an **explicitly chosen** name (set via rename in the metadata panel) is frozen — the app never rewrites it. Every **app-picked** name (the timestamp default, a Chart Builder-generated name, or a previously derived one) is a "next best pick" that keeps tracking the spec: on each publish it is re-derived as the spec's `title` when present, else a mark + encodings description (the same dialect the Chart Builder names its output in), else the existing name stands. So the library reads by chart rather than by creation time, until the user takes over a name — at which point their word is final (see _Spec Editor → Publish_, _Data Model → `nameSource`_). - Each snippet stores a list of **tags**. Tags are persisted and carried through duplication; for example, snippets brought in via import are tagged "imported" (see _Import & Export_). - There is no dedicated tag-management UI; tags are stored on the data model but are not surfaced as a primary browsing or editing control. diff --git a/docs/spec/03-editor-and-drafts.md b/docs/spec/03-editor-and-drafts.md index bfd6475..2b0efe5 100644 --- a/docs/spec/03-editor-and-drafts.md +++ b/docs/spec/03-editor-and-drafts.md @@ -45,6 +45,7 @@ Every snippet carries two versions of its spec: a **published** (stable) version - A **Publish** action promotes the current draft to become the published version (the two are made identical). - Publish is also triggered by the keyboard shortcut Cmd/Ctrl+S. +- If the snippet's name is **app-picked** (never explicitly renamed by the user — the timestamp default, a builder-generated name, or an earlier derived one), publish re-derives it from the now-published content: the spec's `title` verbatim when present (string, line array, or `{ text }` forms), else a mark + encodings description in the Chart Builder's naming dialect (e.g. "Bar chart of count by Ship Mode"), else the existing name stands. A name the user has set is never rewritten (see _Snippet Library → Naming & Tags_). - The snippet's dataset references track the draft continuously (recomputed on auto-save, extract, and revert), so publish needs no special reference handling — promoting the draft simply carries the already-current references onto the published version (see _Datasets_ for reference linking). - A success toast confirms the snippet was published. - Publish is unavailable when no snippet is active. diff --git a/docs/spec/09-data-model.md b/docs/spec/09-data-model.md index fcc5d1c..8f77b69 100644 --- a/docs/spec/09-data-model.md +++ b/docs/spec/09-data-model.md @@ -8,19 +8,20 @@ All data lives entirely in the browser. There is no server, account, or sync. Re A **Snippet** is a saved Vega-Lite specification together with its metadata. Snippets are the primary user-authored entity, listed and managed in the _Snippet Library_. -| Field | Type | Meaning | -| ------------- | -------------------- | ------------------------------------------------------------------------------------------------------------------- | -| `id` | string | Unique, stable identifier for the snippet. | -| `version` | number | Schema version of this record, used for read-time migration (see _Schema versioning_ below). | -| `name` | string | Human-readable title shown in the library. | -| `created` | ISO-timestamp string | When the snippet was first created. | -| `modified` | ISO-timestamp string | When the snippet was last saved. | -| `spec` | JSON value | The **published** Vega-Lite spec. May be an object or a string. This is the version rendered and shared by default. | -| `draftSpec` | JSON value | The **working draft** Vega-Lite spec being edited. May be an object or a string. | -| `comment` | string | Free-form user note about the snippet. | -| `tags` | string[] | User-assigned labels for filtering and organization. | -| `datasetRefs` | string[] | Names of _Datasets_ referenced by this spec (see relationships below). | -| `meta` | object | Free-form, extensible metadata bag for app- or feature-specific data. | +| Field | Type | Meaning | +| ------------- | -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `id` | string | Unique, stable identifier for the snippet. | +| `version` | number | Schema version of this record, used for read-time migration (see _Schema versioning_ below). | +| `name` | string | Human-readable title shown in the library. | +| `nameSource` | `'auto' \| 'user'`? | Name provenance: `auto` names keep tracking the spec on publish; `user` names are frozen (see _Snippet Library → Naming & Tags_). Optional — absent on records predating the field, which are treated as `user` unless the name is provably app-picked: the recognizable timestamp default, or identical to what the app derives from the record's own published spec. | +| `created` | ISO-timestamp string | When the snippet was first created. | +| `modified` | ISO-timestamp string | When the snippet was last saved. | +| `spec` | JSON value | The **published** Vega-Lite spec. May be an object or a string. This is the version rendered and shared by default. | +| `draftSpec` | JSON value | The **working draft** Vega-Lite spec being edited. May be an object or a string. | +| `comment` | string | Free-form user note about the snippet. | +| `tags` | string[] | User-assigned labels for filtering and organization. | +| `datasetRefs` | string[] | Names of _Datasets_ referenced by this spec (see relationships below). | +| `meta` | object | Free-form, extensible metadata bag for app- or feature-specific data. | ### Dual spec / draftSpec model diff --git a/src/app/components/SnippetLibrary.test.tsx b/src/app/components/SnippetLibrary.test.tsx index 4477496..25a9e41 100644 --- a/src/app/components/SnippetLibrary.test.tsx +++ b/src/app/components/SnippetLibrary.test.tsx @@ -89,6 +89,66 @@ describe('SnippetLibrary metadata panel (spec §02)', () => { expect(useSnippetStore.getState().snippets[0].name).toBe('Renamed'); }); + test('a publish-derived rename is adopted by the panel, not reverted by its auto-save', () => { + // Regression: the panel's local name state lagged a publish rename, so its + // debounced auto-save wrote the stale default back — the rename flickered + // for ~400ms in the list and then undid itself. + vi.useFakeTimers(); + const s = createSnippet({ id: 'a', now: new Date('2026-01-01T00:00:00Z') }); // auto name + useSnippetStore.getState().hydrate([s], 'a'); + + act(() => { + root.render(); + }); + + act(() => { + useSnippetStore + .getState() + .updateDraft( + JSON.stringify({ + mark: 'bar', + encoding: { x: { field: 'Region' }, y: { aggregate: 'count' } }, + }), + ); + useSnippetStore.getState().publish(new Date('2026-02-01T00:00:00Z')); + }); + + expect(nameInput().value).toBe('Bar chart of count by Region'); + act(() => { + vi.advanceTimersByTime(1000); // any pending auto-save settles + }); + expect(useSnippetStore.getState().snippets[0].name).toBe('Bar chart of count by Region'); + }); + + test('a name edit in progress survives a publish rename (user text wins)', () => { + vi.useFakeTimers(); + const s = createSnippet({ id: 'a', now: new Date('2026-01-01T00:00:00Z') }); + useSnippetStore.getState().hydrate([s], 'a'); + + act(() => { + root.render(); + }); + + act(() => typeInto(nameInput(), 'My Chart')); // diverged, debounce pending + act(() => { + useSnippetStore + .getState() + .updateDraft( + JSON.stringify({ + mark: 'bar', + encoding: { x: { field: 'Region' }, y: { aggregate: 'count' } }, + }), + ); + useSnippetStore.getState().publish(); + }); + + expect(nameInput().value).toBe('My Chart'); // not clobbered by the derived name + act(() => { + vi.advanceTimersByTime(1000); + }); + expect(useSnippetStore.getState().snippets[0].name).toBe('My Chart'); + }); + test('does not loop on a search/sort state change (render-loop guard)', async () => { // A selector that returned a fresh filtered array would re-render forever // (MEMORY → "Zustand stable selectors"); the component derives via useMemo. diff --git a/src/app/components/SnippetLibrary.tsx b/src/app/components/SnippetLibrary.tsx index ecb283a..fb2db1d 100644 --- a/src/app/components/SnippetLibrary.tsx +++ b/src/app/components/SnippetLibrary.tsx @@ -99,6 +99,19 @@ function SnippetMeta({ const [name, setName] = useState(snippet.name); const [comment, setCommentLocal] = useState(snippet.comment); + // The store can rename underneath this panel (publish's content-derived + // naming, spec §03D). Adopt the new store name unless the local field has + // diverged — i.e. the user is mid-edit, and their text wins. Without this, + // the stale local value differs from the store and the auto-save below + // writes the old name right back, silently undoing the publish rename. + const lastStoreName = useRef(snippet.name); + useEffect(() => { + if (snippet.name === lastStoreName.current) return; + const previous = lastStoreName.current; + lastStoreName.current = snippet.name; + setName((local) => (local === previous ? snippet.name : local)); + }, [snippet.name]); + useEffect(() => { if (name === snippet.name) return; const t = setTimeout(() => renameSnippet(snippet.id, name), META_AUTOSAVE_MS); diff --git a/src/app/stores/ChartBuilderStore.ts b/src/app/stores/ChartBuilderStore.ts index 95b2b5b..f41a68e 100644 --- a/src/app/stores/ChartBuilderStore.ts +++ b/src/app/stores/ChartBuilderStore.ts @@ -538,6 +538,9 @@ export const useChartBuilderStore = create((set, get) => ({ // to its dataset (§09F) without extra wiring. Provenance kept in meta (§06). useSnippetStore.getState().createSnippet({ name, + // Generated, not chosen: stays in the auto naming tier, so publish keeps + // the name tracking the spec until the user renames (spec §02 → Naming). + nameSource: 'auto', spec: specText, now, meta: { createdWith: 'chart-builder', builtFromDataset: config.datasetName }, diff --git a/src/app/stores/SnippetStore.test.ts b/src/app/stores/SnippetStore.test.ts index fe7f21c..a178648 100644 --- a/src/app/stores/SnippetStore.test.ts +++ b/src/app/stores/SnippetStore.test.ts @@ -225,6 +225,98 @@ describe('editorView + selectShownText (spec §03D)', () => { }); }); +describe('publish — content-derived naming (untouched default names only)', () => { + const namableSpec = JSON.stringify({ + mark: 'bar', + encoding: { x: { field: 'Ship Mode' }, y: { aggregate: 'count' } }, + }); + + test('publishing upgrades an untouched timestamp name to a content-derived one', () => { + const a = createSnippet({ id: 'a', now: new Date('2026-01-01T00:00:00Z') }); + store().hydrate([a], 'a'); + + store().updateDraft(namableSpec); + store().publish(new Date('2026-02-01T00:00:00Z')); + + expect(selectActiveSnippet(store())?.name).toBe('Bar chart of count by Ship Mode'); + }); + + test('a user-chosen name is never overwritten', () => { + const a = createSnippet({ id: 'a', name: 'My Chart' }); + store().hydrate([a], 'a'); + + store().updateDraft(namableSpec); + store().publish(); + + expect(selectActiveSnippet(store())?.name).toBe('My Chart'); + }); + + test('an undescribable spec keeps the default name', () => { + const a = createSnippet({ id: 'a', now: new Date('2026-01-01T00:00:00Z') }); + store().hydrate([a], 'a'); + + store().updateDraft('{"a":2}'); + store().publish(); + + expect(selectActiveSnippet(store())?.name).toBe(a.name); + }); + + test('an auto name keeps tracking the spec across publishes', () => { + const a = createSnippet({ id: 'a', now: new Date('2026-01-01T00:00:00Z') }); + store().hydrate([a], 'a'); + + store().updateDraft(namableSpec); + store().publish(); + expect(selectActiveSnippet(store())?.name).toBe('Bar chart of count by Ship Mode'); + + store().updateDraft( + JSON.stringify({ + mark: 'line', + encoding: { x: { field: 'date' }, y: { aggregate: 'sum', field: 'revenue' } }, + }), + ); + store().publish(); + expect(selectActiveSnippet(store())?.name).toBe('Line chart of sum of revenue by date'); + }); + + test('a legacy record whose name matches its own derivation stays in the auto tier', () => { + // Records written before nameSource existed: a name identical to what the + // app derives from the published spec is provably app-picked, so adding a + // title and publishing must adopt it (not stay frozen). + const legacy = { + ...createSnippet({ id: 'a', spec: namableSpec, now: new Date('2026-01-01T00:00:00Z') }), + name: 'Bar chart of count by Ship Mode', + nameSource: undefined, + }; + store().hydrate([legacy], 'a'); + + store().updateDraft( + JSON.stringify({ + title: 'Shipments by mode', + mark: 'bar', + encoding: { x: { field: 'Ship Mode' }, y: { aggregate: 'count' } }, + }), + ); + store().publish(); + + expect(selectActiveSnippet(store())?.name).toBe('Shipments by mode'); + }); + + test('an explicit rename freezes the name against later publishes', () => { + const a = createSnippet({ id: 'a', now: new Date('2026-01-01T00:00:00Z') }); + store().hydrate([a], 'a'); + + store().updateDraft(namableSpec); + store().publish(); + store().renameSnippet('a', 'My Chart'); + + store().updateDraft(JSON.stringify({ title: 'Something else', mark: 'bar' })); + store().publish(); + + expect(selectActiveSnippet(store())?.name).toBe('My Chart'); + }); +}); + describe('publish — datasetRefs recomputation', () => { const refSpec = (name: string) => JSON.stringify({ data: { name }, mark: 'bar' }); diff --git a/src/app/stores/SnippetStore.ts b/src/app/stores/SnippetStore.ts index 4c7f6a7..afc9a92 100644 --- a/src/app/stores/SnippetStore.ts +++ b/src/app/stores/SnippetStore.ts @@ -20,7 +20,9 @@ import { create } from 'zustand'; import { createSnippet, + deriveSnippetName, duplicateSnippet as duplicateSnippetRecord, + isAutoNamed, type CreateSnippetOptions, type Snippet, } from '@core/snippet'; @@ -260,7 +262,12 @@ export const useSnippetStore = create((set, get) => ({ if (!target || target.name === name) return s; // unknown id or no change const modified = (now ?? new Date()).toISOString(); return { - snippets: s.snippets.map((x) => (x.id === id ? { ...x, name, modified } : x)), + // An explicit rename freezes the name (`nameSource: 'user'`) — publish's + // content-derived naming only ever rewrites auto-picked names (spec §02 + // → Naming & Tags). + snippets: s.snippets.map((x) => + x.id === id ? { ...x, name, nameSource: 'user' as const, modified } : x, + ), }; }); }, @@ -424,9 +431,16 @@ export const useSnippetStore = create((set, get) => ({ s.id === activeSnippetId ? // Promote the draft and recompute datasetRefs from the now-published // spec, so the bidirectional snippet↔dataset link mirrors reality - // (spec §03D, docs/architecture/07 §3). + // (spec §03D, docs/architecture/07 §3). An auto-picked name keeps + // tracking the published content — title, else mark + encodings — + // and stays auto so the next publish tracks again; a user-chosen + // name (`nameSource: 'user'`) is never rewritten (spec §03D → + // Publish, §02 → Naming & Tags). { ...s, + ...(isAutoNamed(s) + ? { name: deriveSnippetName(s.draftSpec) ?? s.name, nameSource: 'auto' as const } + : {}), spec: s.draftSpec, datasetRefs: recomputeDatasetRefs(s.draftSpec), modified, diff --git a/src/core/import-normalize.test.ts b/src/core/import-normalize.test.ts index c4dee65..c46fc24 100644 --- a/src/core/import-normalize.test.ts +++ b/src/core/import-normalize.test.ts @@ -157,6 +157,22 @@ describe('normalizeImport — snippet normalization', () => { expect(s.tags).not.toContain('imported'); }); + it('preserves name provenance from our own envelopes, drops anything else', () => { + const parsed = [ + currentSnippetRecord({ nameSource: 'auto' }), + currentSnippetRecord({ nameSource: 'user' }), + { ...currentSnippetRecord(), nameSource: 'bogus' }, // foreign value + currentSnippetRecord(), // absent + ]; + const result = normalizeImport(parsed, { now: FIXED_NOW }); + expect(result.snippets.map((s) => s.nameSource)).toEqual([ + 'auto', + 'user', + undefined, + undefined, + ]); + }); + it('fills missing fields on a current snippet with sensible fallbacks', () => { const parsed = [{ created: '2025-03-03T03:03:03.000Z', spec: '{"mark":"line"}' }]; const result = normalizeImport(parsed, { now: FIXED_NOW, makeId: counterIds() }); diff --git a/src/core/import-normalize.ts b/src/core/import-normalize.ts index 6e03ceb..743ed87 100644 --- a/src/core/import-normalize.ts +++ b/src/core/import-normalize.ts @@ -139,6 +139,9 @@ function normalizeSnippet(raw: unknown, nowIso: string, makeId: () => string): S id: typeof r.id === 'string' && r.id !== '' ? r.id : makeId(), version: CURRENT_SNIPPET_VERSION, name: typeof r.name === 'string' ? r.name : 'Untitled', + // Preserve name provenance from our own envelopes; anything else stays + // undefined → isAutoNamed's conservative timestamp-shape fallback. + ...(r.nameSource === 'auto' || r.nameSource === 'user' ? { nameSource: r.nameSource } : {}), created, modified, spec, diff --git a/src/core/snippet.test.ts b/src/core/snippet.test.ts index 4eee904..f79a687 100644 --- a/src/core/snippet.test.ts +++ b/src/core/snippet.test.ts @@ -2,10 +2,13 @@ import { describe, expect, test } from 'vitest'; import { CURRENT_SNIPPET_VERSION, createSnippet, + deriveSnippetName, duplicateSnippet, formatSnippetSize, generateSnippetName, hasUnpublishedChanges, + isAutoNamed, + isDefaultSnippetName, SAMPLE_SPEC, sampleSpecText, snippetSizeBytes, @@ -68,6 +71,136 @@ describe('generateSnippetName', () => { }); }); +describe('isDefaultSnippetName', () => { + test('recognizes an untouched auto-generated name', () => { + expect(isDefaultSnippetName(generateSnippetName(new Date(2026, 0, 5, 9, 7, 3)))).toBe(true); + }); + + test('rejects user-chosen names, including near-misses', () => { + expect(isDefaultSnippetName('My Chart')).toBe(false); + expect(isDefaultSnippetName('Snippet 2026-01-05')).toBe(false); // no time part + expect(isDefaultSnippetName('Snippet 2026-01-05 09:07:03 v2')).toBe(false); // user suffix + }); +}); + +describe('name provenance (nameSource / isAutoNamed)', () => { + test('createSnippet stamps provenance: default name → auto, explicit name → user', () => { + expect(createSnippet({ id: 'a' }).nameSource).toBe('auto'); + expect(createSnippet({ id: 'b', name: 'My Chart' }).nameSource).toBe('user'); + }); + + test('a generator can pass a derived name that stays in the auto tier', () => { + const s = createSnippet({ id: 'a', name: 'Bar chart of x by y', nameSource: 'auto' }); + expect(isAutoNamed(s)).toBe(true); + }); + + test('isAutoNamed proves provenance for legacy records (no nameSource)', () => { + // Records persisted before nameSource existed carry no provenance; only + // provably app-picked shapes count as auto. + const derivable = JSON.stringify({ + mark: 'bar', + encoding: { x: { field: 'Ship Mode' }, y: { aggregate: 'count' } }, + }); + expect(isAutoNamed({ name: 'Snippet 2026-01-05 09:07:03', spec: '{}' })).toBe(true); + // Name identical to what the app derives from the published spec → auto. + expect(isAutoNamed({ name: 'Bar chart of count by Ship Mode', spec: derivable })).toBe(true); + // Anything else is conservatively user-chosen. + expect(isAutoNamed({ name: 'My Chart', spec: derivable })).toBe(false); + expect(isAutoNamed({ name: 'Bar chart of count by Ship Mode', spec: '{}' })).toBe(false); + }); + + test('duplicateSnippet carries the source provenance onto the copy', () => { + const auto = createSnippet({ id: 'a' }); + const user = createSnippet({ id: 'b', name: 'My Chart' }); + expect(duplicateSnippet(auto, { id: 'a2' }).nameSource).toBe('auto'); + expect(duplicateSnippet(user, { id: 'b2' }).nameSource).toBe('user'); + }); +}); + +describe('deriveSnippetName (content-based library names)', () => { + const spec = (body: object) => JSON.stringify(body); + + test('a spec-level title wins verbatim', () => { + expect(deriveSnippetName(spec({ title: ' Quarterly revenue ', mark: 'bar' }))).toBe( + 'Quarterly revenue', + ); + }); + + test("title's object and multi-line forms collapse to one line", () => { + expect(deriveSnippetName(spec({ title: { text: 'Revenue', subtitle: 'FY26' } }))).toBe( + 'Revenue', + ); + expect(deriveSnippetName(spec({ title: ['Revenue', 'by quarter'] }))).toBe( + 'Revenue by quarter', + ); + expect(deriveSnippetName(spec({ title: { text: ['Revenue', 'by quarter'] } }))).toBe( + 'Revenue by quarter', + ); + }); + + test('mark + x/y encodings read like the Chart Builder dialect', () => { + expect( + deriveSnippetName( + spec({ + mark: 'bar', + encoding: { + x: { field: 'Ship Mode', type: 'nominal' }, + y: { aggregate: 'count' }, + }, + }), + ), + ).toBe('Bar chart of count by Ship Mode'); + }); + + test('aggregates phrase as " of " / "unique "', () => { + expect( + deriveSnippetName( + spec({ + mark: 'line', + encoding: { + x: { field: 'date', type: 'temporal' }, + y: { aggregate: 'sum', field: 'revenue' }, + }, + }), + ), + ).toBe('Line chart of sum of revenue by date'); + expect( + deriveSnippetName( + spec({ + mark: 'point', + encoding: { + x: { field: 'region' }, + y: { aggregate: 'distinct', field: 'customer' }, + }, + }), + ), + ).toBe('Point chart of unique customer by region'); + }); + + test('an object mark contributes its type; a single channel still names', () => { + expect( + deriveSnippetName( + spec({ + mark: { type: 'arc', tooltip: true }, + encoding: { theta: { field: 'share', type: 'quantitative' } }, + }), + ), + ).toBe('Arc chart of share'); + }); + + test('returns null when there is nothing to describe', () => { + expect(deriveSnippetName('not json {')).toBeNull(); + expect(deriveSnippetName('[]')).toBeNull(); + expect(deriveSnippetName(spec({}))).toBeNull(); // no mark + expect(deriveSnippetName(spec({ mark: 'bar' }))).toBeNull(); // no encodings + expect(deriveSnippetName(spec({ mark: 'bar', encoding: { x: { value: 5 } } }))).toBeNull(); // constants aren't names + }); + + test('the sample template derives a sensible name', () => { + expect(deriveSnippetName(sampleSpecText())).toBe('Bar chart of value by category'); + }); +}); + describe('duplicateSnippet', () => { const source = { ...createSnippet({ diff --git a/src/core/snippet.ts b/src/core/snippet.ts index 1ba9ff9..ee26afb 100644 --- a/src/core/snippet.ts +++ b/src/core/snippet.ts @@ -28,6 +28,14 @@ export interface Snippet { version: number; /** Human-readable title shown in the library. */ name: string; + /** + * Name provenance — the naming hierarchy's gate. `'user'`: explicitly chosen + * (rename / metadata panel) — frozen, never rewritten. `'auto'`: app-picked + * (timestamp default, builder-generated, or publish-derived) — keeps tracking + * the spec's content on publish. Absent on records from before the field + * existed; `isAutoNamed` then falls back to recognizing the timestamp shape. + */ + nameSource?: 'auto' | 'user'; /** ISO timestamp — when first created. */ created: string; /** ISO timestamp — when last saved. */ @@ -92,9 +100,117 @@ export function generateSnippetName(now: Date): string { return `Snippet ${date} ${time}`; } +/** Whether `name` is (still) an untouched `generateSnippetName` auto-default. */ +export function isDefaultSnippetName(name: string): boolean { + return /^Snippet \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/.test(name.trim()); +} + +/** + * Whether the snippet's name is app-picked (rewritable on publish) rather than + * user-chosen (frozen). Records predating `nameSource` carry no provenance, so + * the rule is: never rewrite a name we can't prove the user didn't choose. Two + * shapes are provable — the timestamp default, and a name identical to what + * the app derives from the snippet's own published spec (only the generator + * produces that string for that spec; a user typing it verbatim is naming the + * content, which is what tracking preserves). + */ +export function isAutoNamed(snippet: Pick): boolean { + if (snippet.nameSource !== undefined) return snippet.nameSource === 'auto'; + if (isDefaultSnippetName(snippet.name)) return true; + return deriveSnippetName(snippet.spec) === snippet.name; +} + +/** A channel's encoding definition, as far as naming cares about it. */ +interface EncodingDef { + field?: unknown; + aggregate?: unknown; + value?: unknown; +} + +/** A human phrase for what an encoding channel shows, e.g. "sum of revenue". */ +function describeEncoding(def: EncodingDef): string | null { + if (def.value !== undefined) return null; // a constant — nothing to name + const aggregate = typeof def.aggregate === 'string' ? def.aggregate : undefined; + if (aggregate === 'count') return 'count'; + const field = typeof def.field === 'string' ? def.field.trim() : ''; + if (field === '') return null; // repeat refs / missing field — not nameable + if (aggregate === 'distinct') return `unique ${field}`; + if (aggregate) return `${aggregate} of ${field}`; + return field; +} + +/** + * Derive a descriptive name from a spec's content — `"Bar chart of by "` + * (the same dialect as the Chart Builder's `generateChartName`, so manually + * authored and builder-built snippets read alike in the library; NN/g #6 + * recognition-over-recall). A spec-level `title` wins verbatim. Returns `null` + * when the spec is unparseable or carries too little to describe (no usable + * mark/encodings) — callers keep the existing name then. + */ +export function deriveSnippetName(specText: string): string | null { + let spec: unknown; + try { + spec = JSON.parse(specText); + } catch { + return null; + } + if (typeof spec !== 'object' || spec === null || Array.isArray(spec)) return null; + const s = spec as Record; + + // Vega-Lite titles are a string, an array of lines, or a params object whose + // `text` is either; all collapse to one line here. + const titleText = (value: unknown): string => { + if (typeof value === 'string') return value.trim(); + if (Array.isArray(value)) + return value + .filter((line): line is string => typeof line === 'string') + .map((line) => line.trim()) + .filter(Boolean) + .join(' '); + if (typeof value === 'object' && value !== null) + return titleText((value as { text?: unknown }).text); + return ''; + }; + const title = titleText(s.title); + if (title) return title; + + const markRaw = + typeof s.mark === 'string' + ? s.mark + : typeof s.mark === 'object' && s.mark !== null + ? (s.mark as { type?: unknown }).type + : undefined; + if (typeof markRaw !== 'string' || markRaw === '') return null; + const mark = markRaw.charAt(0).toUpperCase() + markRaw.slice(1); + + const encoding = + typeof s.encoding === 'object' && s.encoding !== null + ? (s.encoding as Record) + : {}; + const phraseFor = (channel: string): string | null => { + const def = encoding[channel]; + if (typeof def !== 'object' || def === null) return null; + return describeEncoding(def); + }; + + const x = phraseFor('x'); + const y = phraseFor('y'); + if (x && y) return `${mark} chart of ${y} by ${x}`; + const only = x ?? y ?? phraseFor('theta') ?? phraseFor('color'); + if (only) return `${mark} chart of ${only}`; + return null; +} + export interface CreateSnippetOptions { /** Override the auto-generated name. */ name?: string; + /** + * Name provenance override. Defaults to `'user'` when `name` is given (an + * explicit name is presumed chosen) and `'auto'` for the timestamp default; + * generators passing a derived `name` (the Chart Builder) say `'auto'` so the + * name keeps tracking the spec until the user renames. + */ + nameSource?: 'auto' | 'user'; /** Override the starting spec text (defaults to the sample template). */ spec?: string; /** Clock injection for deterministic tests; defaults to the current time. */ @@ -117,6 +233,7 @@ export function createSnippet(options: CreateSnippetOptions = {}): Snippet { id: options.id ?? crypto.randomUUID(), version: CURRENT_SNIPPET_VERSION, name: options.name ?? generateSnippetName(now), + nameSource: options.nameSource ?? (options.name !== undefined ? 'user' : 'auto'), created: iso, modified: iso, spec,