diff --git a/docs/architecture/07-naming-and-relationships.md b/docs/architecture/07-naming-and-relationships.md index d234970..bfcfab2 100644 --- a/docs/architecture/07-naming-and-relationships.md +++ b/docs/architecture/07-naming-and-relationships.md @@ -224,38 +224,56 @@ export function recomputeDatasetRefs(spec: Json): string[] { ## 4. Reverse lookup: who uses this dataset? The Dataset Manager shows a **usage badge** and a **Linked Snippets** list; the -Snippet Library shows a snippet's linked datasets. Both come from one selector -scan — no stored back-pointer to drift. +Snippet Library shows a snippet's linked datasets. Both come from one scan — no +stored back-pointer to drift. + +The scan itself is **pure** and lives in core, taking the snippets as a parameter +so the _same_ implementation serves two callers: the reactive UI (which passes its +live `snippets` selection straight in) and the non-reactive service wrapper (which +passes a store snapshot for programmatic callers). This is what makes "who +references this" have exactly one implementation (§6). ```ts -// src/app/services/RelationshipService.ts - -import { useSnippetStore } from '../stores/SnippetStore'; -import type { Snippet } from '../../core/types'; +// src/core/relationships.ts — pure scan (unit-tested) /** Snippets whose datasetRefs include `name` (case-insensitive). */ -export function findSnippetsReferencingDataset(name: string): Snippet[] { +export function snippetsReferencingDataset( + snippets: readonly T[], + name: string, +): T[] { const lower = name.toLowerCase(); - return useSnippetStore - .getState() - .snippets.filter((s) => s.datasetRefs.some((ref) => ref.toLowerCase() === lower)); + return snippets.filter((s) => s.datasetRefs.some((ref) => ref.toLowerCase() === lower)); } -/** Count for the usage badge. */ -export function datasetUsageCount(name: string): number { - return findSnippetsReferencingDataset(name).length; +/** Usage counts for the whole library in one pass, keyed by lower-cased name. */ +export function datasetUsageCounts( + snippets: readonly { datasetRefs: readonly string[] }[], +): Map; +``` + +```ts +// src/app/services/RelationshipService.ts — snapshot wrapper for non-reactive callers + +import { snippetsReferencingDataset } from '../../core/relationships'; +import { useSnippetStore } from '../stores/SnippetStore'; + +export function findSnippetsReferencingDataset(name: string): Snippet[] { + return snippetsReferencingDataset(useSnippetStore.getState().snippets, name); } ``` -Because this reads `useSnippetStore.getState().snippets`, exposing it as a -selector for the UI makes the badge and Linked Snippets list reactive for free — -they update the moment any snippet is published with changed refs. +The reactive UI does **not** go through the service: a component subscribed to +`snippets` calls the core helper directly, so the badge and Linked Snippets list +update the moment any snippet is published with changed refs — no duplicated +matching logic, no `getState()` snapshot that would miss updates. **Do** -- Keep reverse lookup a pure scan over the store. It is O(snippets) but the - collections are small (library budget ~5 MB); clarity beats an index. -- Expose it as a selector where the UI needs reactivity. +- Keep reverse lookup a pure scan, parameterized on the snippets so both the + reactive and snapshot callers share it. It is O(snippets) but the collections + are small (library budget ~5 MB); clarity beats an index. +- Call the core helper directly from a store-subscribed component for reactivity; + use the service wrapper only from non-reactive (snapshot) code. **Don't** @@ -464,14 +482,15 @@ export function renameDatasetEverywhere(oldName: string, newName: string): { upd ## 7. Where things live -| Concern | Location | Pure? | Tested | -| --------------------------------------------- | ----------------------------------------- | ------------------------------ | ---------------- | -| `makeUniqueName`, `isNameTaken` | `src/core/naming.ts` | yes | unit | -| `extractDatasetRefs`, `recomputeDatasetRefs` | `src/core/spec-refs.ts` | yes | unit | -| `renameDatasetInSpec` | `src/core/spec-refs.ts` | yes | unit | -| `findSnippetsReferencingDataset`, usage count | `src/app/services/RelationshipService.ts` | no (reads store) | integration | -| `renameDatasetEverywhere` | `src/app/services/RelationshipService.ts` | no (mutates stores) | integration | -| `dedupeIncomingDatasetNames` | `src/app/services/ImportService.ts` | nearly (uses `makeUniqueName`) | unit/integration | +| Concern | Location | Pure? | Tested | +| ------------------------------------------------------------------------ | ----------------------------------------- | ------------------------------ | ---------------- | +| `makeUniqueName`, `isNameTaken` | `src/core/naming.ts` | yes | unit | +| `extractDatasetRefs`, `recomputeDatasetRefs` | `src/core/spec-refs.ts` | yes | unit | +| `renameDatasetInSpec` | `src/core/spec-refs.ts` | yes | unit | +| `snippetsReferencingDataset`, `datasetUsageCounts` (reverse-lookup scan) | `src/core/relationships.ts` | yes | unit | +| `findSnippetsReferencingDataset`, usage count (snapshot wrappers) | `src/app/services/RelationshipService.ts` | no (reads store) | integration | +| `renameDatasetEverywhere` → `{ updated }` | `src/app/services/RelationshipService.ts` | no (mutates stores) | integration | +| `dedupeIncomingDatasetNames` | `src/app/services/ImportService.ts` | nearly (uses `makeUniqueName`) | unit/integration | 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/src/app/components/DatasetsModal.tsx b/src/app/components/DatasetsModal.tsx index d1c1158..40e07de 100644 --- a/src/app/components/DatasetsModal.tsx +++ b/src/app/components/DatasetsModal.tsx @@ -16,6 +16,7 @@ import { useState } from 'react'; import { useShallow } from 'zustand/react/shallow'; import { datasetReference, type DataSource, type Dataset } from '@core/dataset'; import { detectFormat, detectFormatFromUrl, type DataFormat } from '@core/format-detection'; +import { datasetUsageCounts, snippetsReferencingDataset } from '@core/relationships'; import { closeModal, openModal, resnapshot } from '../modals/ModalCoordinator'; import { confirm } from '../stores/ConfirmStore'; import { notify } from '../stores/NotificationStore'; @@ -44,18 +45,6 @@ function humanBytes(bytes: number): string { return `${mb < 10 ? mb.toFixed(1) : Math.round(mb)} MB`; } -/** Map of dataset name (lower-cased) → how many snippets reference it. */ -function usageByName(snippets: ReadonlyArray<{ datasetRefs: string[] }>): Map { - const counts = new Map(); - for (const s of snippets) { - for (const ref of s.datasetRefs) { - const key = ref.toLowerCase(); - counts.set(key, (counts.get(key) ?? 0) + 1); - } - } - return counts; -} - const SOURCE_OPTIONS: ReadonlyArray> = [ { value: 'inline', label: 'Inline' }, { value: 'url', label: 'URL' }, @@ -70,7 +59,7 @@ export function DatasetsModal() { const select = useDatasetStore((s) => s.select); const startCreate = useDatasetStore((s) => s.startCreate); - const usage = usageByName(snippets); + const usage = datasetUsageCounts(snippets); const ordered = [...datasets].sort(byModifiedDesc); const handleNew = () => { @@ -165,8 +154,7 @@ function DatasetDetail({ const selectSnippet = useSnippetStore((s) => s.selectSnippet); const [copied, setCopied] = useState(false); - const lower = dataset.name.toLowerCase(); - const linked = snippets.filter((s) => s.datasetRefs.some((r) => r.toLowerCase() === lower)); + const linked = snippetsReferencingDataset(snippets, dataset.name); const handleEdit = () => { startEdit(); diff --git a/src/app/services/RelationshipService.test.ts b/src/app/services/RelationshipService.test.ts index c87e1e9..8676013 100644 --- a/src/app/services/RelationshipService.test.ts +++ b/src/app/services/RelationshipService.test.ts @@ -45,23 +45,21 @@ describe('reverse lookup', () => { describe('renameDatasetEverywhere', () => { test('renames the dataset record and every referencing snippet', () => { - useDatasetStore - .getState() - .add( - createDataset({ - name: 'Sales', - data: [{ a: 1 }], - format: 'json', - source: 'inline', - now: T, - }), - ); + useDatasetStore.getState().add( + createDataset({ + name: 'Sales', + data: [{ a: 1 }], + format: 'json', + source: 'inline', + now: T, + }), + ); useSnippetStore.getState().hydrate([seedSnippet('a', 'Sales')], 'a'); useSnippetStore.getState().publish(T); - const updated = renameDatasetEverywhere('Sales', 'Revenue', new Date('2026-07-01T00:00:00Z')); + const result = renameDatasetEverywhere('Sales', 'Revenue', new Date('2026-07-01T00:00:00Z')); - expect(updated).toBe(1); + expect(result).toEqual({ updated: 1 }); expect(useDatasetStore.getState().datasets[0].name).toBe('Revenue'); const s = useSnippetStore.getState().snippets[0]; expect(s.datasetRefs).toEqual(['Revenue']); @@ -69,6 +67,6 @@ describe('renameDatasetEverywhere', () => { }); test('is a no-op when old and new names are equal', () => { - expect(renameDatasetEverywhere('Sales', 'Sales')).toBe(0); + expect(renameDatasetEverywhere('Sales', 'Sales')).toEqual({ updated: 0 }); }); }); diff --git a/src/app/services/RelationshipService.ts b/src/app/services/RelationshipService.ts index c8f6fbd..cb655ec 100644 --- a/src/app/services/RelationshipService.ts +++ b/src/app/services/RelationshipService.ts @@ -13,25 +13,19 @@ */ import type { Snippet } from '@core/snippet'; +import { snippetsReferencingDataset } from '@core/relationships'; import { useDatasetStore } from '../stores/DatasetStore'; import { useSnippetStore } from '../stores/SnippetStore'; -// TODO: nothing in production calls this service yet — DatasetsModal derives -// usage/linked-snippets inline (it must read `snippets` reactively, not via the -// getState snapshot these use), and DatasetStore.save propagates rename itself -// (it updates other fields besides the name, so it can't delegate cleanly). -// `renameDatasetEverywhere` also matches by exact name, unlike the rest of the -// case-insensitive naming policy, and arch/07 documents it returning `{ updated }` -// not a bare number. Either reconcile the duplication (extract a reactive-friendly -// pure `snippetsReferencing(snippets, name)` helper both sides share) or fold this -// service in when M4's "Build Chart from dataset" needs a non-reactive caller. +// The reverse-lookup scan itself is the pure `snippetsReferencingDataset` +// (`core/relationships`) — the *one* implementation arch/07 §6 calls for. These +// wrappers just bind it to a store snapshot for non-reactive callers (e.g. a +// programmatic rename); the reactive UI calls the core helper directly with its +// live `snippets` selection, so neither side duplicates the matching logic. -/** Snippets whose `datasetRefs` include `name` (case-insensitive). */ +/** Snippets whose `datasetRefs` include `name` (case-insensitive), from the store. */ export function findSnippetsReferencingDataset(name: string): Snippet[] { - const lower = name.toLowerCase(); - return useSnippetStore - .getState() - .snippets.filter((s) => s.datasetRefs.some((ref) => ref.toLowerCase() === lower)); + return snippetsReferencingDataset(useSnippetStore.getState().snippets, name); } /** Count for the dataset usage badge (spec §05 → List item). */ @@ -42,13 +36,18 @@ export function datasetUsageCount(name: string): number { /** * Rename a dataset and propagate everywhere (docs/architecture/07 §6): renames * the dataset record, then rewrites every referencing snippet's spec, draftSpec, - * and datasetRefs. The caller is responsible for collision policy on `newName` - * (the edit form rejects a taken name; programmatic paths pre-resolve a unique - * one). Returns the number of snippets updated. + * and datasetRefs (via `SnippetStore.renameDatasetRefs`, the single rename impl). + * The caller is responsible for collision policy on `newName` (the edit form + * rejects a taken name; programmatic paths pre-resolve a unique one). Returns the + * number of snippets updated, as `{ updated }` (arch/07 §6). */ -export function renameDatasetEverywhere(oldName: string, newName: string, now?: Date): number { - if (oldName === newName) return 0; +export function renameDatasetEverywhere( + oldName: string, + newName: string, + now?: Date, +): { updated: number } { + if (oldName === newName) return { updated: 0 }; const dataset = useDatasetStore.getState().datasets.find((d) => d.name === oldName); if (dataset) useDatasetStore.getState().update(dataset.id, { name: newName }, now); - return useSnippetStore.getState().renameDatasetRefs(oldName, newName, now); + return { updated: useSnippetStore.getState().renameDatasetRefs(oldName, newName, now) }; } diff --git a/src/core/relationships.test.ts b/src/core/relationships.test.ts new file mode 100644 index 0000000..3764f4d --- /dev/null +++ b/src/core/relationships.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, test } from 'vitest'; +import { datasetUsageCounts, snippetsReferencingDataset } from './relationships'; + +/** A minimal snippet-like record — the helpers only read `datasetRefs`. */ +const snip = (id: string, datasetRefs: string[]) => ({ id, datasetRefs }); + +describe('snippetsReferencingDataset', () => { + const snippets = [ + snip('a', ['Sales']), + snip('b', ['SALES', 'Regions']), + snip('c', ['Other']), + snip('d', []), + ]; + + test('matches case-insensitively and preserves the input records', () => { + expect(snippetsReferencingDataset(snippets, 'sales').map((s) => s.id)).toEqual(['a', 'b']); + // The matched element is the same object that was passed in (no remapping). + expect(snippetsReferencingDataset(snippets, 'sales')[0]).toBe(snippets[0]); + }); + + test('returns an empty array when nothing references the name', () => { + expect(snippetsReferencingDataset(snippets, 'Nonexistent')).toEqual([]); + }); +}); + +describe('datasetUsageCounts', () => { + test('counts every reference, keyed by lower-cased name, in one pass', () => { + const counts = datasetUsageCounts([ + snip('a', ['Sales']), + snip('b', ['SALES', 'Regions']), + snip('c', ['regions']), + ]); + expect(counts.get('sales')).toBe(2); + expect(counts.get('regions')).toBe(2); + expect(counts.get('other')).toBeUndefined(); + }); + + test('is empty for a library with no references', () => { + expect(datasetUsageCounts([snip('a', []), snip('b', [])]).size).toBe(0); + }); +}); diff --git a/src/core/relationships.ts b/src/core/relationships.ts new file mode 100644 index 0000000..5d0a707 --- /dev/null +++ b/src/core/relationships.ts @@ -0,0 +1,46 @@ +/** + * Snippet → dataset reverse lookup (docs/architecture/07 §4). + * + * The forward link is a snippet's `datasetRefs` (recomputed from its published + * spec). The reverse — "which snippets use this dataset" — is **derived** by a + * scan, never stored, so it can't drift. This module is that scan's single pure + * implementation: it takes plain snippet records and a name and returns plain + * data, so the same logic serves both the reactive UI (the Dataset Manager passes + * its live `snippets` selection) and the non-reactive `RelationshipService` (which + * passes a store snapshot). One definition, per arch/07 §6 ("'who references this' + * has exactly one implementation"). + * + * Matching is **case-insensitive**, mirroring the naming policy (`core/naming`): + * a reference to `"sales"` and a dataset named `"Sales"` are the same link. + */ + +/** The minimal snippet shape the reverse lookup needs (its forward refs). */ +interface HasDatasetRefs { + datasetRefs: readonly string[]; +} + +/** Snippets whose `datasetRefs` include `name` (case-insensitive). */ +export function snippetsReferencingDataset( + snippets: readonly T[], + name: string, +): T[] { + const lower = name.toLowerCase(); + return snippets.filter((s) => s.datasetRefs.some((ref) => ref.toLowerCase() === lower)); +} + +/** + * Usage counts for every referenced dataset name, keyed by **lower-cased** name — + * one pass over the library for the Dataset Manager's per-row badges (cheaper than + * an O(snippets) `snippetsReferencingDataset` scan per dataset). Look up a dataset + * with `counts.get(name.toLowerCase()) ?? 0`. + */ +export function datasetUsageCounts(snippets: readonly HasDatasetRefs[]): Map { + const counts = new Map(); + for (const s of snippets) { + for (const ref of s.datasetRefs) { + const key = ref.toLowerCase(); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + } + return counts; +}