From 827411f25462a05c022b8e54cb0f6d0fe1fcc9cc Mon Sep 17 00:00:00 2001 From: robbond Date: Thu, 3 Sep 2026 18:29:00 +0100 Subject: [PATCH] refactor(confidence-engine): v0.60d clarify investigation storage identity Replace bare re-export in investigation-storage.js with explicit wrapper functions that own the canonical identity contract: snapshot.id is the sole save identity authority. The provider never allocates or changes IDs. 7 new deterministic tests prove: identified snapshots persist under their own id key, explicit competing id arguments are ignored, A/B remain independently addressable, unknown IDs return null, and legacy singleton compatibility is preserved for unmigrated callers. No application callers modified. UI/routes not migrated. --- docs/current-handoff.md | 34 +++++++ lib/storage/investigation-storage.js | 37 +++++++- tests/storage/investigation-storage.test.js | 98 +++++++++++++++++++++ 3 files changed, 165 insertions(+), 4 deletions(-) diff --git a/docs/current-handoff.md b/docs/current-handoff.md index 9d27672..400a2d8 100644 --- a/docs/current-handoff.md +++ b/docs/current-handoff.md @@ -349,6 +349,40 @@ Migrate existing application callers to the identity-aware contract signatures: 3. `scenario-form.jsx` — pass investigation ID through save calls 4. `app/page.jsx` — migrate Portfolio to use `listInvestigations()` (next increment) +## v0.60d — Canonical Save Identity Contract + +**Problem:** `investigation-storage.js` was a bare re-export (`export { ... } from "./providers/local-storage.js"`). It did not own semantic contract — whatever signatures the provider exposed were what consumers received. The provider accepted an independent explicit `id` argument that could silently override or compete with `snapshot.id`. + +**Decision:** `investigation-storage.js` now owns the canonical identity contract. It wraps the provider with explicit semantics: + +- Canonical save identity comes solely from `investigation.id`; +- `investigation-storage.js` owns application-facing semantics; +- localStorage provider remains implementation detail (never allocated ID, never changed it); +- Any remaining singleton compatibility behaviour is explicitly temporary — for unmigrated callers only. + +**Implementation:** + +| Module | Role | +|---|---| +| `lib/storage/investigation-storage.js` | Application-facing boundary — owns identity contract | +| `saveInvestigation(snapshot, explicitId)` | Uses `snapshot.id` as sole save key when present; falls back to singleton path for unidentified legacy snapshots | +| `loadInvestigation(id)` | Passes raw id to provider (identity-aware) or singleton path (legacy) | +| `lib/storage/providers/local-storage.js` | Concrete localStorage implementation — unchanged, representation remains private | + +**Test:** 7 new deterministic tests in `tests/storage/investigation-storage.test.js` under describe block "v0.60d canonical identity" prove: + +1. Identified snapshot persists under its own id key (not CANONICAL_KEY) +2. Load by same id round-trips correctly +3. Provider does not invent or replace the supplied ID +4. Explicit competing id argument is silently ignored when snapshot has an id +5. Two identified investigations (A/B) remain independently addressable +6. Unknown ID returns null +7. Unidentified legacy snapshots still fall back to CANONICAL_KEY + +**Status:** UI/routes are **not** migrated. All existing callers continue via the singleton compatibility path (they call `saveInvestigation({ ... })` with no explicit second parameter, and their snapshots carry no `id` field). No production caller was modified in this increment. + +**Next restart point:** Caller migration — update application consumers to pass investigation ID through save calls so canonical identity-aware semantics activate for all writes. + ## Next implementation boundary Smallest next increment: implement the four-operation storage contract in `lib/storage/investigation-storage.js` as a re-export of a provider-backed interface whose signatures accept/return domain Investigation objects keyed by durable ID — without committing to any specific localStorage or database representation. This means defining the exported function signatures and the Investigation shape that flows through them, while deferring key scheme, row schema, and collection structure to a later implementation decision. diff --git a/lib/storage/investigation-storage.js b/lib/storage/investigation-storage.js index 67b6995..64479e2 100644 --- a/lib/storage/investigation-storage.js +++ b/lib/storage/investigation-storage.js @@ -1,5 +1,34 @@ -// investigation-storage — generic persistence boundary -// Exposes loadInvestigation / saveInvestigation / clearInvestigation -// and delegates internally to the concrete localStorage provider. +// investigation-storage — application-facing persistence boundary +// Owns the canonical identity contract: snapshot.id is the sole save identity. +// Delegates to the concrete localStorage provider internally. -export { loadInvestigation, saveInvestigation, clearInvestigation } from "./providers/local-storage.js"; +import { loadInvestigation as _load, saveInvestigation as _save, clearInvestigation as _clear } from "./providers/local-storage.js"; + +/** + * Canonical save contract: snapshot.id is the sole identity authority. + * When snapshot carries an id — persist under that id key (identity-aware). + * When snapshot has no id — fall back to legacy singleton compatibility path. + */ +export function saveInvestigation(snapshot, explicitId) { + if (snapshot && typeof snapshot === "object" && snapshot.id != null) { + return _save(snapshot, snapshot.id); + } + // Legacy unidentified snapshot — singleton fallback for unmigrated callers + return _save(snapshot, explicitId); +} + +/** + * Canonical load contract: select by durable id when supplied; + * fall back to legacy singleton path otherwise. + */ +export function loadInvestigation(id) { + return _load(id != null ? id : undefined); +} + +/** + * Canonical clear contract: remove by identity-aware key when supplied; + * fall back to legacy singleton keys otherwise. + */ +export function clearInvestigation(id) { + return _clear(id ?? undefined); +} diff --git a/tests/storage/investigation-storage.test.js b/tests/storage/investigation-storage.test.js index c1ff2cf..3b0e776 100644 --- a/tests/storage/investigation-storage.test.js +++ b/tests/storage/investigation-storage.test.js @@ -333,3 +333,101 @@ describe("investigation-storage v0.60c identity-aware", () => { }); }); + +// ── v0.60d — canonical save identity contract (snapshot.id is sole authority) ─ + +describe("investigation-storage v0.60d canonical identity", () => { + + function newId() { + return `inv-${Math.random().toString(36).slice(2, 9)}`; + } + + // Import the application-facing wrapper (not the bare provider) + async function getWrapper() { + vi.resetModules(); + const m = await import("../../lib/storage/investigation-storage.js"); + return { load: m.loadInvestigation, save: m.saveInvestigation, clear: m.clearInvestigation }; + } + + beforeEach(() => { + globalThis.window.localStorage.clear(); + }); + + // ── 1. saveInvestigation({ id: "inv-a", ... }) persists under inv-a ── + + it("save with snapshot.id persists under that id key", async () => { + const { save } = await getWrapper(); + const snap = makeSnapshot({ id: newId(), title: "Alpha" }); + save(snap); + const raw = globalThis.window.localStorage.getItem(`confidence-engine-investigation:${snap.id}`); + expect(raw).not.toBeNull(); + }); + + // ── 2. loadInvestigation("inv-a") returns inv-a ─────────────── + + it("save with snapshot.id and load by same id round-trips", async () => { + const { load, save } = await getWrapper(); + const snap = makeSnapshot({ id: newId(), title: "Alpha" }); + save(snap); + const loaded = load(snap.id); + expect(loaded).not.toBeNull(); + expect(loaded.id).toBe(snap.id); + expect(loaded.title).toBe("Alpha"); + }); + + // ── 3. provider does not allocate/change the ID ──────────────── + + it("saveInvestigation does not invent or replace snapshot.id", async () => { + const { save } = await getWrapper(); + const snap = makeSnapshot({ id: "inv-audit-99", title: "Audit" }); + save(snap); + const raw = globalThis.window.localStorage.getItem(`confidence-engine-investigation:${snap.id}`); + expect(JSON.parse(raw).id).toBe("inv-audit-99"); + }); + + // ── 4. canonical save has no second competing identity authority ─ + + it("explicit id argument is ignored when snapshot has an id", async () => { + const { load, save } = await getWrapper(); + const snap = makeSnapshot({ id: newId(), title: "Alpha" }); + const fakeSecondId = "fake-override"; + save(snap, fakeSecondId); + // Should NOT appear under fakeSecondId — identity comes from snapshot.id only + const fakeRaw = globalThis.window.localStorage.getItem(`confidence-engine-investigation:${fakeSecondId}`); + expect(fakeRaw).toBeNull(); + }); + + // ── 5. A and B remain independently addressable ──────────────── + + it("save two identified investigations are independent", async () => { + const { load, save } = await getWrapper(); + const idA = newId(); + const idB = newId(); + const snapA = makeSnapshot({ id: idA, title: "Alpha" }); + const snapB = makeSnapshot({ id: idB, title: "Beta" }); + save(snapA); + save(snapB); + expect((await load(idA)).id).toBe(idA); + expect((await load(idA)).title).toBe("Alpha"); + expect((await load(idB)).id).toBe(idB); + expect((await load(idB)).title).toBe("Beta"); + }); + + // ── 6. unknown ID remains null ──────────────────────────────── + + it("loadInvestigation(unknownId) returns null", async () => { + const { load } = await getWrapper(); + expect(await load("non-existent-id-xyz")).toBeNull(); + }); + + // ── 7. legacy singleton compatibility preserved ─────────────── + + it("unidentified snapshot falls back to CANONICAL_KEY for legacy callers", async () => { + const { load, save } = await getWrapper(); + save(makeSnapshot({ legacy: true })); // no id on snapshot + const loaded = load(); // no id argument + expect(loaded).not.toBeNull(); + expect(loaded.legacy).toBe(true); + }); + +});