From 730ed12d3cf10f114fbca1f2e7b1d4ed5debe2b5 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 11:26:21 +0100 Subject: [PATCH] Address follow-up review suggestions for XState roadmap integration - Move deleteFeature actor cleanup outside set() for consistency - Skip no-op store writes when XState silently ignores events - Use 'as const' on assign action string literals for type narrowing Co-Authored-By: Claude Opus 4.6 --- .../src/renderer/stores/roadmap-store.ts | 22 +++++++++++-------- .../state-machines/roadmap-feature-machine.ts | 12 +++++----- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/apps/frontend/src/renderer/stores/roadmap-store.ts b/apps/frontend/src/renderer/stores/roadmap-store.ts index 81ae7358..638e45b2 100644 --- a/apps/frontend/src/renderer/stores/roadmap-store.ts +++ b/apps/frontend/src/renderer/stores/roadmap-store.ts @@ -265,6 +265,9 @@ export const useRoadmapStore = create((set) => ({ const derivedStatus = mapFeatureStateToStatus(String(snapshot.value)); const ctx = snapshot.context; + // Skip store write if XState silently ignored the event (no-op transition) + if (derivedStatus === feature.status && ctx.taskOutcome === feature.taskOutcome && ctx.previousStatus === feature.previousStatus) return; + set((s) => { if (!s.roadmap) return s; const updatedFeatures = s.roadmap.features.map((f) => @@ -363,17 +366,17 @@ export const useRoadmapStore = create((set) => ({ }); }, - deleteFeature: (featureId) => + deleteFeature: (featureId) => { + // Stop and remove the feature's actor outside set() + const actor = featureActors.get(featureId); + if (actor) { + actor.stop(); + featureActors.delete(featureId); + } + set((state) => { if (!state.roadmap) return state; - // Stop and remove the feature's actor - const actor = featureActors.get(featureId); - if (actor) { - actor.stop(); - featureActors.delete(featureId); - } - const updatedFeatures = state.roadmap.features.filter( (feature) => feature.id !== featureId ); @@ -385,7 +388,8 @@ export const useRoadmapStore = create((set) => ({ updatedAt: new Date() } }; - }), + }); + }, clearRoadmap: () => { // Stop all actors and clear Maps diff --git a/apps/frontend/src/shared/state-machines/roadmap-feature-machine.ts b/apps/frontend/src/shared/state-machines/roadmap-feature-machine.ts index 0e01bae0..277970e1 100644 --- a/apps/frontend/src/shared/state-machines/roadmap-feature-machine.ts +++ b/apps/frontend/src/shared/state-machines/roadmap-feature-machine.ts @@ -174,12 +174,12 @@ export const roadmapFeatureMachine = createMachine( linkedSpecId: ({ event }) => event.type === 'LINK_SPEC' ? event.specId : undefined }), - savePreviousUnderReview: assign({ previousStatus: () => 'under_review' }), - savePreviousPlanned: assign({ previousStatus: () => 'planned' }), - savePreviousInProgress: assign({ previousStatus: () => 'in_progress' }), - setTaskOutcomeCompleted: assign({ taskOutcome: () => 'completed' }), - setTaskOutcomeDeleted: assign({ taskOutcome: () => 'deleted' }), - setTaskOutcomeArchived: assign({ taskOutcome: () => 'archived' }), + savePreviousUnderReview: assign({ previousStatus: () => 'under_review' as const }), + savePreviousPlanned: assign({ previousStatus: () => 'planned' as const }), + savePreviousInProgress: assign({ previousStatus: () => 'in_progress' as const }), + setTaskOutcomeCompleted: assign({ taskOutcome: () => 'completed' as const }), + setTaskOutcomeDeleted: assign({ taskOutcome: () => 'deleted' as const }), + setTaskOutcomeArchived: assign({ taskOutcome: () => 'archived' as const }), clearDoneContext: assign({ taskOutcome: () => undefined, previousStatus: () => undefined