From 6a56b94477b4aa85405df817bef7fedc8a7eabbd Mon Sep 17 00:00:00 2001 From: Jan-Henrik Bruhn Date: Mon, 20 Oct 2025 12:24:52 +0200 Subject: [PATCH] refactor: move snapshot creation to historyStore (architectural fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes architectural issue where snapshot logic was duplicated between hook and store layers. Problem Identified: - useDocumentHistory (hook) created snapshots with duplicate logic - timelineStore (store) created snapshots with duplicate logic - timelineStore couldn't call useDocumentHistory (hooks can't be used in stores) - Snapshot creation logic scattered across codebase Root Cause: Timeline operations happen in the store layer but history was managed via a hook. This architectural mismatch forced duplication. Solution: - Add pushToHistory() method to historyStore - Single source of truth for snapshot creation - Both hook and store layers call historyStore.pushToHistory() - Eliminates all snapshot creation duplication Changes: - Add historyStore.pushToHistory() - high-level API with snapshot creation - Keep historyStore.pushAction() - low-level API (for future use) - Update useDocumentHistory to call historyStore.pushToHistory() - Update timelineStore pushDocumentHistory() to call historyStore.pushToHistory() - Remove createDocumentSnapshot import from useDocumentHistory - Remove createDocumentSnapshot import from timelineStore Architecture: Before: Hook (useDocumentHistory) → creates snapshot → historyStore.pushAction() Store (timelineStore) → creates snapshot → historyStore.pushAction() [Duplication at 2 call sites] After: Hook (useDocumentHistory) → historyStore.pushToHistory() → creates snapshot Store (timelineStore) → historyStore.pushToHistory() → creates snapshot [Single implementation in historyStore] Benefits: - ✅ Zero snapshot creation duplication - ✅ Proper architectural separation (store handles snapshots, not hook/store) - ✅ Single source of truth (historyStore.pushToHistory) - ✅ Timeline and graph operations use identical snapshot logic - ✅ Easy to modify snapshot behavior in future (one place) Impact: - No breaking changes - No behavior changes - Better code organization - Easier maintenance Related: Phase 2.1 architectural improvement 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/hooks/useDocumentHistory.ts | 18 +++----------- src/stores/historyStore.ts | 44 +++++++++++++++++++++++++++++++-- src/stores/timelineStore.ts | 25 ++++++++----------- 3 files changed, 55 insertions(+), 32 deletions(-) diff --git a/src/hooks/useDocumentHistory.ts b/src/hooks/useDocumentHistory.ts index b8fbdcd..2695bc0 100644 --- a/src/hooks/useDocumentHistory.ts +++ b/src/hooks/useDocumentHistory.ts @@ -4,7 +4,6 @@ import { useWorkspaceStore } from '../stores/workspaceStore'; import { useHistoryStore } from '../stores/historyStore'; import { useGraphStore } from '../stores/graphStore'; import { useTimelineStore } from '../stores/timelineStore'; -import { createDocumentSnapshot } from '../stores/workspace/documentUtils'; import type { DocumentSnapshot } from '../stores/historyStore'; /** @@ -59,25 +58,14 @@ export function useDocumentHistory() { return; } - // ✅ Use centralized snapshot creation (single source of truth) - const snapshot = createDocumentSnapshot( + // ✅ Call historyStore's high-level pushToHistory (single source of truth) + historyStore.pushToHistory( activeDocumentId, + description, activeDoc, timeline, graphStore ); - - if (!snapshot) { - console.warn('Failed to create snapshot'); - return; - } - - // Push to history - historyStore.pushAction(activeDocumentId, { - description, - timestamp: Date.now(), - documentState: snapshot, - }); }, [activeDocumentId, historyStore] ); diff --git a/src/stores/historyStore.ts b/src/stores/historyStore.ts index 74d996f..fc78e47 100644 --- a/src/stores/historyStore.ts +++ b/src/stores/historyStore.ts @@ -1,6 +1,9 @@ import { create } from "zustand"; import type { NodeTypeConfig, EdgeTypeConfig, LabelConfig } from "../types"; -import type { ConstellationState, StateId } from "../types/timeline"; +import type { ConstellationState, StateId, Timeline } from "../types/timeline"; +import type { Actor, Relation, Group } from "../types"; +import type { ConstellationDocument } from "./persistence/types"; +import { createDocumentSnapshot } from "./workspace/documentUtils"; /** * History Store - Per-Document Undo/Redo System @@ -52,9 +55,18 @@ interface HistoryActions { // Initialize history for a document initializeHistory: (documentId: string) => void; - // Push a new action onto the document's history stack + // Push a new action onto the document's history stack (low-level) pushAction: (documentId: string, action: HistoryAction) => void; + // Push action with automatic snapshot creation (high-level - USE THIS) + pushToHistory: ( + documentId: string, + description: string, + document: ConstellationDocument, + timeline: Timeline, + graphStore: { nodes: Actor[]; edges: Relation[]; groups: Group[] } + ) => void; + // Undo the last action for a specific document undo: (documentId: string, currentSnapshot: DocumentSnapshot) => DocumentSnapshot | null; @@ -163,6 +175,34 @@ export const useHistoryStore = create( }); }, + pushToHistory: ( + documentId: string, + description: string, + document: ConstellationDocument, + timeline: Timeline, + graphStore: { nodes: Actor[]; edges: Relation[]; groups: Group[] } + ) => { + // ✅ Use centralized snapshot creation (single source of truth) + const snapshot = createDocumentSnapshot( + documentId, + document, + timeline, + graphStore + ); + + if (!snapshot) { + console.warn('Failed to create snapshot for history'); + return; + } + + // Call low-level pushAction + get().pushAction(documentId, { + description, + timestamp: Date.now(), + documentState: snapshot, + }); + }, + undo: (documentId: string, currentSnapshot: DocumentSnapshot) => { const state = get(); const history = state.histories.get(documentId); diff --git a/src/stores/timelineStore.ts b/src/stores/timelineStore.ts index c8520bc..ae66a01 100644 --- a/src/stores/timelineStore.ts +++ b/src/stores/timelineStore.ts @@ -11,7 +11,6 @@ import { useGraphStore } from "./graphStore"; import { useWorkspaceStore } from "./workspaceStore"; import { useToastStore } from "./toastStore"; import { useHistoryStore } from "./historyStore"; -import { createDocumentSnapshot } from "./workspace/documentUtils"; /** * Timeline Store @@ -33,7 +32,13 @@ function generateStateId(): StateId { return `state_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; } -// Helper to push document state to history +/** + * Helper to push document state to history from timeline operations + * + * This is a convenience wrapper that gathers the required state and calls + * historyStore.pushToHistory(). Timeline operations use this to track changes + * in the unified document history system. + */ function pushDocumentHistory(documentId: string, description: string) { const historyStore = useHistoryStore.getState(); const timelineStore = useTimelineStore.getState(); @@ -48,24 +53,14 @@ function pushDocumentHistory(documentId: string, description: string) { return; } - // ✅ Use centralized snapshot creation (single source of truth) - const snapshot = createDocumentSnapshot( + // ✅ Call historyStore's high-level pushToHistory (single source of truth for snapshots) + historyStore.pushToHistory( documentId, + description, document, timeline, graphStore ); - - if (!snapshot) { - console.warn('Failed to create snapshot'); - return; - } - - historyStore.pushAction(documentId, { - description, - timestamp: Date.now(), - documentState: snapshot, - }); } export const useTimelineStore = create(