Make DatasetStore the collision-free id authority

src/app/stores/DatasetStore.ts

@@ -67,14 +67,19 @@ export interface DatasetState {
    */
   save: (now?: Date) => boolean;
 
-  /** Low-level: add a fully-formed dataset and select it. */
+  /**
+   * Low-level: add a fully-formed dataset and select it. The id is (re)assigned
+   * via `nextDatasetId`, so a `createDataset` default id (`Date.now()`) can never
+   * reach storage — see `nextDatasetId` for why that matters.
+   */
   add: (dataset: Dataset) => void;
   /**
    * Append imported datasets (spec §08 → datasets imported before snippets). Each
-   * is given a fresh monotonic numeric id so a batch never collides on the
-   * `Date.now()` default (the `add` TODO) nor with existing ids — safe because
-   * datasets are referenced by **name**, not id (docs/architecture/07 §1). Names
-   * are assumed already de-duped by the import service. Selection is unchanged.
+   * is given a fresh monotonic numeric id (`nextDatasetId`, the same authority as
+   * `add`) so a batch never collides on the `Date.now()` default nor with existing
+   * ids — safe because datasets are referenced by **name**, not id
+   * (docs/architecture/07 §1). Names are assumed already de-duped by the import
+   * service. Selection is unchanged.
    */
   addDatasets: (incoming: Dataset[]) => void;
   /** Low-level: merge a patch into a dataset, advancing `modified`. */
@@ -91,6 +96,20 @@ export function byModifiedDesc(a: Dataset, b: Dataset): number {
   return b.modified.localeCompare(a.modified);
 }
 
+/**
+ * Next free numeric id for a dataset: one past the current maximum. This store is
+ * the **single id authority** — both single (`add`) and batch (`addDatasets`)
+ * insertion route through it — so a `createDataset` default id (`Date.now()`)
+ * never reaches IndexedDB, where a tight creation loop (the documented import path)
+ * could repeat the same millisecond and collide on the numeric key. Dataset ids
+ * are an internal IndexedDB key only — snippets reference datasets by **name**
+ * (docs/architecture/07 §1) — so reusing an id freed by a deletion is harmless,
+ * which is why one-past-the-max suffices without a persistent counter.
+ */
+function nextDatasetId(datasets: ReadonlyArray<Dataset>): number {
+  return datasets.reduce((max, d) => Math.max(max, d.id), 0) + 1;
+}
+
 /**
  * Cheap form validation: returns an error message, or `null` when the form is
  * saveable. Deliberately does **not** `JSON.parse` the input — it only checks
@@ -256,18 +275,16 @@ export const useDatasetStore = create<DatasetState>((set, get) => ({
     return true;
   },
 
-  // TODO: createDataset ids default to `Date.now()` and `add` does not reassign
-  // on collision (despite that file's comment). Interactive create is safe, but
-  // the documented non-interactive import path (naming.ts) creates many datasets
-  // in a tight loop where `Date.now()` repeats — duplicate numeric keys would
-  // collide in IndexedDB. Give the store a monotonic id source (or reassign here)
-  // before wiring import.
-  add: (dataset) => set((s) => ({ datasets: [dataset, ...s.datasets], selectedId: dataset.id })),
+  add: (dataset) =>
+    set((s) => {
+      const withId = { ...dataset, id: nextDatasetId(s.datasets) };
+      return { datasets: [withId, ...s.datasets], selectedId: withId.id };
+    }),
 
   addDatasets: (incoming) => {
     if (incoming.length === 0) return;
     set((s) => {
-      let nextId = s.datasets.reduce((max, d) => Math.max(max, d.id), 0) + 1;
+      let nextId = nextDatasetId(s.datasets);
       const withIds = incoming.map((d) => ({ ...d, id: nextId++ }));
       return { datasets: [...withIds, ...s.datasets] };
     });