feat: improve label deletion atomicity with immutable timeline updates

Implements Phase 5 of the state management refactoring plan.

Problem:
The previous implementation mutated timeline states in place when
removing label references from nodes and edges. This created a risk
of partial state corruption if an error occurred mid-operation.

Solution:
Refactored deleteLabelFromDocument to use immutable updates:

1. Build new timeline states Map with cleaned labels
   - Map over all states, nodes, and edges immutably
   - Create new objects instead of mutating existing ones

2. Atomic timeline swap
   - Replace entire timeline structure at once
   - Either fully succeeds or fully fails (no partial state)

3. Simplified rollback
   - Capture entire original timeline (shallow copy)
   - Restore complete timeline on failure
   - No need to patch individual nodes/edges

Benefits:
-  True atomicity - timeline updates are all-or-nothing
-  Cleaner rollback - restore entire structure, not individual items
-  Reduced mutation risk - immutable pattern throughout
-  Better data integrity across timeline states
-  Consistent with React/Zustand best practices

The operation now builds a completely new timeline structure and
swaps it atomically, ensuring that label deletion either fully
succeeds across all timeline states or fully rolls back.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Jan-Henrik Bruhn 2025-10-20 15:10:14 +02:00
parent d03be68860
commit 60748a2235

View file

@ -1352,74 +1352,72 @@ export const useWorkspaceStore = create<Workspace & WorkspaceActions>((set, get)
const originalLabels = [...doc.labels]; const originalLabels = [...doc.labels];
const originalIsDirty = state.documentMetadata.get(documentId)?.isDirty; const originalIsDirty = state.documentMetadata.get(documentId)?.isDirty;
// Capture original timeline state (deep copy of nodes/edges that contain the label) // Capture original timeline for rollback (shallow copy of the entire timeline)
const timelineStore = useTimelineStore.getState(); const timelineStore = useTimelineStore.getState();
const timeline = timelineStore.timelines.get(documentId); const timeline = timelineStore.timelines.get(documentId);
const originalTimelineStates = new Map<string, { const originalTimeline = timeline ? { ...timeline, states: new Map(timeline.states) } : null;
nodes: Array<{ id: string; labels: string[] }>;
edges: Array<{ id: string; labels: string[] }>;
}>();
if (timeline) {
timeline.states.forEach((constellationState, stateId) => {
const affectedNodes: Array<{ id: string; labels: string[] }> = [];
const affectedEdges: Array<{ id: string; labels: string[] }> = [];
// Capture nodes that have this label
constellationState.graph.nodes.forEach((node) => {
const nodeData = node.data as { labels?: string[] };
if (nodeData?.labels && nodeData.labels.includes(labelId)) {
affectedNodes.push({ id: node.id, labels: [...nodeData.labels] });
}
});
// Capture edges that have this label
constellationState.graph.edges.forEach((edge) => {
const edgeData = edge.data as { labels?: string[] };
if (edgeData?.labels && edgeData.labels.includes(labelId)) {
affectedEdges.push({ id: edge.id, labels: [...edgeData.labels] });
}
});
if (affectedNodes.length > 0 || affectedEdges.length > 0) {
originalTimelineStates.set(stateId, { nodes: affectedNodes, edges: affectedEdges });
}
});
}
get().executeTypeTransaction( get().executeTypeTransaction(
() => { () => {
// 1. Remove from document's labels // 1. Remove from document's labels
doc.labels = (doc.labels || []).filter((label) => label.id !== labelId); doc.labels = (doc.labels || []).filter((label) => label.id !== labelId);
// 2. Remove label from all nodes and edges in all timeline states // 2. Remove label from all nodes and edges in all timeline states (IMMUTABLE)
if (timeline) { if (timeline) {
// ✅ Build new states Map with cleaned labels (immutable update)
const newStates = new Map();
let hasChanges = false; let hasChanges = false;
// Iterate through all timeline states and clean up label references timeline.states.forEach((constellationState, stateId) => {
timeline.states.forEach((constellationState) => { // Create new state with cleaned labels
// Clean up nodes const cleanedState = {
constellationState.graph.nodes.forEach((node) => { ...constellationState,
graph: {
...constellationState.graph,
nodes: constellationState.graph.nodes.map((node) => {
const nodeData = node.data as { labels?: string[] }; const nodeData = node.data as { labels?: string[] };
if (nodeData?.labels && nodeData.labels.includes(labelId)) { if (nodeData?.labels?.includes(labelId)) {
nodeData.labels = nodeData.labels.filter((id: string) => id !== labelId);
hasChanges = true; hasChanges = true;
return {
...node,
data: {
...nodeData,
labels: nodeData.labels.filter((id: string) => id !== labelId),
},
};
} }
}); return node;
}),
// Clean up edges edges: constellationState.graph.edges.map((edge) => {
constellationState.graph.edges.forEach((edge) => {
const edgeData = edge.data as { labels?: string[] }; const edgeData = edge.data as { labels?: string[] };
if (edgeData?.labels && edgeData.labels.includes(labelId)) { if (edgeData?.labels?.includes(labelId)) {
edgeData.labels = edgeData.labels.filter((id: string) => id !== labelId);
hasChanges = true; hasChanges = true;
return {
...edge,
data: {
...edgeData,
labels: edgeData.labels.filter((id: string) => id !== labelId),
},
};
} }
}); return edge;
}),
},
};
newStates.set(stateId, cleanedState);
}); });
// If this is the active document and changes were made, sync to graphStore // ✅ Atomic swap - replace entire timeline at once
if (hasChanges && documentId === state.activeDocumentId) { if (hasChanges) {
const currentState = timeline.states.get(timeline.currentStateId); const newTimeline = {
...timeline,
states: newStates,
};
timelineStore.timelines.set(documentId, newTimeline);
// Sync to graphStore if active
if (documentId === state.activeDocumentId) {
const currentState = newStates.get(timeline.currentStateId);
if (currentState) { if (currentState) {
useGraphStore.setState({ useGraphStore.setState({
nodes: currentState.graph.nodes as Actor[], nodes: currentState.graph.nodes as Actor[],
@ -1428,6 +1426,7 @@ export const useWorkspaceStore = create<Workspace & WorkspaceActions>((set, get)
} }
} }
} }
}
// 3. Save document to storage (can throw QuotaExceededError) // 3. Save document to storage (can throw QuotaExceededError)
saveDocumentToStorage(documentId, doc); saveDocumentToStorage(documentId, doc);
@ -1444,38 +1443,13 @@ export const useWorkspaceStore = create<Workspace & WorkspaceActions>((set, get)
// Rollback on failure // Rollback on failure
doc.labels = originalLabels; doc.labels = originalLabels;
// Restore timeline state label references // Restore entire timeline (atomic rollback)
if (timeline) { if (originalTimeline) {
originalTimelineStates.forEach((originalState, stateId) => { timelineStore.timelines.set(documentId, originalTimeline);
const constellationState = timeline.states.get(stateId);
if (!constellationState) return;
// Restore node labels
originalState.nodes.forEach((originalNode) => {
const node = constellationState.graph.nodes.find((n) => n.id === originalNode.id);
if (node) {
const nodeData = node.data as { labels?: string[] };
if (nodeData) {
nodeData.labels = [...originalNode.labels];
}
}
});
// Restore edge labels
originalState.edges.forEach((originalEdge) => {
const edge = constellationState.graph.edges.find((e) => e.id === originalEdge.id);
if (edge) {
const edgeData = edge.data as { labels?: string[] };
if (edgeData) {
edgeData.labels = [...originalEdge.labels];
}
}
});
});
// Sync restored state to graphStore if active // Sync restored state to graphStore if active
if (documentId === state.activeDocumentId) { if (documentId === state.activeDocumentId) {
const currentState = timeline.states.get(timeline.currentStateId); const currentState = originalTimeline.states.get(originalTimeline.currentStateId);
if (currentState) { if (currentState) {
useGraphStore.setState({ useGraphStore.setState({
nodes: currentState.graph.nodes as Actor[], nodes: currentState.graph.nodes as Actor[],