Extract reverse-lookup scan to core/relationships (one shared impl)

This commit is contained in:
2026-06-07 23:04:20 +03:00
parent 39555322e4
commit 1cad9ba140
6 changed files with 167 additions and 76 deletions
@@ -224,38 +224,56 @@ export function recomputeDatasetRefs(spec: Json): string[] {
## 4. Reverse lookup: who uses this dataset? ## 4. Reverse lookup: who uses this dataset?
The Dataset Manager shows a **usage badge** and a **Linked Snippets** list; the 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 Snippet Library shows a snippet's linked datasets. Both come from one scan — no
scan — no stored back-pointer to drift. 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 ```ts
// src/app/services/RelationshipService.ts // src/core/relationships.ts — pure scan (unit-tested)
import { useSnippetStore } from '../stores/SnippetStore';
import type { Snippet } from '../../core/types';
/** Snippets whose datasetRefs include `name` (case-insensitive). */ /** Snippets whose datasetRefs include `name` (case-insensitive). */
export function findSnippetsReferencingDataset(name: string): Snippet[] { export function snippetsReferencingDataset<T extends { datasetRefs: readonly string[] }>(
snippets: readonly T[],
name: string,
): T[] {
const lower = name.toLowerCase(); const lower = name.toLowerCase();
return useSnippetStore return snippets.filter((s) => s.datasetRefs.some((ref) => ref.toLowerCase() === lower));
.getState()
.snippets.filter((s) => s.datasetRefs.some((ref) => ref.toLowerCase() === lower));
} }
/** Count for the usage badge. */ /** Usage counts for the whole library in one pass, keyed by lower-cased name. */
export function datasetUsageCount(name: string): number { export function datasetUsageCounts(
return findSnippetsReferencingDataset(name).length; snippets: readonly { datasetRefs: readonly string[] }[],
): Map<string, number>;
```
```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 The reactive UI does **not** go through the service: a component subscribed to
selector for the UI makes the badge and Linked Snippets list reactive for free — `snippets` calls the core helper directly, so the badge and Linked Snippets list
they update the moment any snippet is published with changed refs. update the moment any snippet is published with changed refs — no duplicated
matching logic, no `getState()` snapshot that would miss updates.
**Do** **Do**
- Keep reverse lookup a pure scan over the store. It is O(snippets) but the - Keep reverse lookup a pure scan, parameterized on the snippets so both the
collections are small (library budget ~5 MB); clarity beats an index. reactive and snapshot callers share it. It is O(snippets) but the collections
- Expose it as a selector where the UI needs reactivity. 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** **Don't**
@@ -465,12 +483,13 @@ export function renameDatasetEverywhere(oldName: string, newName: string): { upd
## 7. Where things live ## 7. Where things live
| Concern | Location | Pure? | Tested | | Concern | Location | Pure? | Tested |
| --------------------------------------------- | ----------------------------------------- | ------------------------------ | ---------------- | | ------------------------------------------------------------------------ | ----------------------------------------- | ------------------------------ | ---------------- |
| `makeUniqueName`, `isNameTaken` | `src/core/naming.ts` | yes | unit | | `makeUniqueName`, `isNameTaken` | `src/core/naming.ts` | yes | unit |
| `extractDatasetRefs`, `recomputeDatasetRefs` | `src/core/spec-refs.ts` | yes | unit | | `extractDatasetRefs`, `recomputeDatasetRefs` | `src/core/spec-refs.ts` | yes | unit |
| `renameDatasetInSpec` | `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 | | `snippetsReferencingDataset`, `datasetUsageCounts` (reverse-lookup scan) | `src/core/relationships.ts` | yes | unit |
| `renameDatasetEverywhere` | `src/app/services/RelationshipService.ts` | no (mutates stores) | integration | | `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 | | `dedupeIncomingDatasetNames` | `src/app/services/ImportService.ts` | nearly (uses `makeUniqueName`) | unit/integration |
The dividing line: anything that takes plain data and returns plain data is The dividing line: anything that takes plain data and returns plain data is
+3 -15
View File
@@ -16,6 +16,7 @@ import { useState } from 'react';
import { useShallow } from 'zustand/react/shallow'; import { useShallow } from 'zustand/react/shallow';
import { datasetReference, type DataSource, type Dataset } from '@core/dataset'; import { datasetReference, type DataSource, type Dataset } from '@core/dataset';
import { detectFormat, detectFormatFromUrl, type DataFormat } from '@core/format-detection'; import { detectFormat, detectFormatFromUrl, type DataFormat } from '@core/format-detection';
import { datasetUsageCounts, snippetsReferencingDataset } from '@core/relationships';
import { closeModal, openModal, resnapshot } from '../modals/ModalCoordinator'; import { closeModal, openModal, resnapshot } from '../modals/ModalCoordinator';
import { confirm } from '../stores/ConfirmStore'; import { confirm } from '../stores/ConfirmStore';
import { notify } from '../stores/NotificationStore'; import { notify } from '../stores/NotificationStore';
@@ -44,18 +45,6 @@ function humanBytes(bytes: number): string {
return `${mb < 10 ? mb.toFixed(1) : Math.round(mb)} MB`; 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<string, number> {
const counts = new Map<string, number>();
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<SegmentedOption<DataSource>> = [ const SOURCE_OPTIONS: ReadonlyArray<SegmentedOption<DataSource>> = [
{ value: 'inline', label: 'Inline' }, { value: 'inline', label: 'Inline' },
{ value: 'url', label: 'URL' }, { value: 'url', label: 'URL' },
@@ -70,7 +59,7 @@ export function DatasetsModal() {
const select = useDatasetStore((s) => s.select); const select = useDatasetStore((s) => s.select);
const startCreate = useDatasetStore((s) => s.startCreate); const startCreate = useDatasetStore((s) => s.startCreate);
const usage = usageByName(snippets); const usage = datasetUsageCounts(snippets);
const ordered = [...datasets].sort(byModifiedDesc); const ordered = [...datasets].sort(byModifiedDesc);
const handleNew = () => { const handleNew = () => {
@@ -165,8 +154,7 @@ function DatasetDetail({
const selectSnippet = useSnippetStore((s) => s.selectSnippet); const selectSnippet = useSnippetStore((s) => s.selectSnippet);
const [copied, setCopied] = useState(false); const [copied, setCopied] = useState(false);
const lower = dataset.name.toLowerCase(); const linked = snippetsReferencingDataset(snippets, dataset.name);
const linked = snippets.filter((s) => s.datasetRefs.some((r) => r.toLowerCase() === lower));
const handleEdit = () => { const handleEdit = () => {
startEdit(); startEdit();
+4 -6
View File
@@ -45,9 +45,7 @@ describe('reverse lookup', () => {
describe('renameDatasetEverywhere', () => { describe('renameDatasetEverywhere', () => {
test('renames the dataset record and every referencing snippet', () => { test('renames the dataset record and every referencing snippet', () => {
useDatasetStore useDatasetStore.getState().add(
.getState()
.add(
createDataset({ createDataset({
name: 'Sales', name: 'Sales',
data: [{ a: 1 }], data: [{ a: 1 }],
@@ -59,9 +57,9 @@ describe('renameDatasetEverywhere', () => {
useSnippetStore.getState().hydrate([seedSnippet('a', 'Sales')], 'a'); useSnippetStore.getState().hydrate([seedSnippet('a', 'Sales')], 'a');
useSnippetStore.getState().publish(T); 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'); expect(useDatasetStore.getState().datasets[0].name).toBe('Revenue');
const s = useSnippetStore.getState().snippets[0]; const s = useSnippetStore.getState().snippets[0];
expect(s.datasetRefs).toEqual(['Revenue']); expect(s.datasetRefs).toEqual(['Revenue']);
@@ -69,6 +67,6 @@ describe('renameDatasetEverywhere', () => {
}); });
test('is a no-op when old and new names are equal', () => { 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 });
}); });
}); });
+19 -20
View File
@@ -13,25 +13,19 @@
*/ */
import type { Snippet } from '@core/snippet'; import type { Snippet } from '@core/snippet';
import { snippetsReferencingDataset } from '@core/relationships';
import { useDatasetStore } from '../stores/DatasetStore'; import { useDatasetStore } from '../stores/DatasetStore';
import { useSnippetStore } from '../stores/SnippetStore'; import { useSnippetStore } from '../stores/SnippetStore';
// TODO: nothing in production calls this service yet — DatasetsModal derives // The reverse-lookup scan itself is the pure `snippetsReferencingDataset`
// usage/linked-snippets inline (it must read `snippets` reactively, not via the // (`core/relationships`) — the *one* implementation arch/07 §6 calls for. These
// getState snapshot these use), and DatasetStore.save propagates rename itself // wrappers just bind it to a store snapshot for non-reactive callers (e.g. a
// (it updates other fields besides the name, so it can't delegate cleanly). // programmatic rename); the reactive UI calls the core helper directly with its
// `renameDatasetEverywhere` also matches by exact name, unlike the rest of the // live `snippets` selection, so neither side duplicates the matching logic.
// 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.
/** Snippets whose `datasetRefs` include `name` (case-insensitive). */ /** Snippets whose `datasetRefs` include `name` (case-insensitive), from the store. */
export function findSnippetsReferencingDataset(name: string): Snippet[] { export function findSnippetsReferencingDataset(name: string): Snippet[] {
const lower = name.toLowerCase(); return snippetsReferencingDataset(useSnippetStore.getState().snippets, name);
return useSnippetStore
.getState()
.snippets.filter((s) => s.datasetRefs.some((ref) => ref.toLowerCase() === lower));
} }
/** Count for the dataset usage badge (spec §05 → List item). */ /** 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 * Rename a dataset and propagate everywhere (docs/architecture/07 §6): renames
* the dataset record, then rewrites every referencing snippet's spec, draftSpec, * the dataset record, then rewrites every referencing snippet's spec, draftSpec,
* and datasetRefs. The caller is responsible for collision policy on `newName` * and datasetRefs (via `SnippetStore.renameDatasetRefs`, the single rename impl).
* (the edit form rejects a taken name; programmatic paths pre-resolve a unique * The caller is responsible for collision policy on `newName` (the edit form
* one). Returns the number of snippets updated. * 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 { export function renameDatasetEverywhere(
if (oldName === newName) return 0; oldName: string,
newName: string,
now?: Date,
): { updated: number } {
if (oldName === newName) return { updated: 0 };
const dataset = useDatasetStore.getState().datasets.find((d) => d.name === oldName); const dataset = useDatasetStore.getState().datasets.find((d) => d.name === oldName);
if (dataset) useDatasetStore.getState().update(dataset.id, { name: newName }, now); 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) };
} }
+41
View File
@@ -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);
});
});
+46
View File
@@ -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<T extends HasDatasetRefs>(
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<string, number> {
const counts = new Map<string, number>();
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;
}