Snippet naming: content-derived on publish, frozen on explicit rename

This commit is contained in:
2026-06-13 10:13:57 +03:00
parent 92bfe888b5
commit 4e5108f434
14 changed files with 508 additions and 16 deletions
@@ -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(<SnippetLibrary />);
});
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(<SnippetLibrary />);
});
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.
+13
View File
@@ -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);
+3
View File
@@ -538,6 +538,9 @@ export const useChartBuilderStore = create<ChartBuilderState>((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 },
+92
View File
@@ -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' });
+16 -2
View File
@@ -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<SnippetState>((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<SnippetState>((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,
+16
View File
@@ -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() });
+3
View File
@@ -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,
+133
View File
@@ -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 "<agg> of <field>" / "unique <field>"', () => {
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({
+117
View File
@@ -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<Snippet, 'name' | 'nameSource' | 'spec'>): 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 <y> by <x>"`
* (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<string, unknown>;
// 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<string, unknown>)
: {};
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,