fix: address second round of review findings for PR #1815

Fixes 6 issues identified in follow-up review (2 HIGH, 1 MEDIUM, 3 LOW):

1. HIGH [65226bd9539f]: Generation can now restart from complete/error states
   - Added RESET + START_GENERATION logic in 'analyzing' case

2. HIGH [282536242c43]: Phase restoration fixed for discovering/generating
   - Drive actor through intermediate transitions to reach target state
   - Handle idle and complete/error states before sending phase events

3. MEDIUM [1b74aa0cd0f1]: Progress value clamping added to updateProgress
   - Clamp progress to 0-100 range using Math.min/max

4. LOW [c833788d76ce]: setError now resets progress to 0 on error
   - Consistent error state with progress reset

5. LOW [686d19af55d5]: Document unused SETTLED_STATES constants
   - Added comments explaining future public API use

6. LOW [605d439c4efd]: Document getState() outside set() pattern
   - Explain XState actors as external side effects

Co-Authored-By: Claude Opus 4.6 <[email protected]>

apps/frontend/src/renderer/stores/roadmap-store.ts

@@ -204,18 +204,40 @@ export const useRoadmapStore = create<RoadmapState>((set) => ({
     // Map the incoming status phase to an XState event
     let event: RoadmapGenerationEvent | null = null;
     switch (status.phase) {
-      case 'analyzing':
-        // If idle, start generation; otherwise it's a progress update
-        if (String(actor.getSnapshot().value) === 'idle') {
+      case 'analyzing': {
+        const currentState = String(actor.getSnapshot().value);
+        if (currentState === 'idle') {
+          event = { type: 'START_GENERATION' };
+        } else if (currentState === 'complete' || currentState === 'error') {
+          actor.send({ type: 'RESET' });
           event = { type: 'START_GENERATION' };
         }
         break;
-      case 'discovering':
+      }
+      case 'discovering': {
+        const cs = String(actor.getSnapshot().value);
+        if (cs === 'idle') {
+          actor.send({ type: 'START_GENERATION' });
+        } else if (cs === 'complete' || cs === 'error') {
+          actor.send({ type: 'RESET' });
+          actor.send({ type: 'START_GENERATION' });
+        }
         event = { type: 'DISCOVERY_STARTED' };
         break;
-      case 'generating':
+      }
+      case 'generating': {
+        const cs = String(actor.getSnapshot().value);
+        if (cs === 'idle') {
+          actor.send({ type: 'START_GENERATION' });
+          actor.send({ type: 'DISCOVERY_STARTED' });
+        } else if (cs === 'complete' || cs === 'error') {
+          actor.send({ type: 'RESET' });
+          actor.send({ type: 'START_GENERATION' });
+          actor.send({ type: 'DISCOVERY_STARTED' });
+        }
         event = { type: 'GENERATION_STARTED' };
         break;
+      }
       case 'complete':
         event = { type: 'GENERATION_COMPLETE' };
         break;
@@ -253,6 +275,10 @@ export const useRoadmapStore = create<RoadmapState>((set) => ({
   setCurrentProjectId: (projectId) => set({ currentProjectId: projectId }),
 
   updateFeatureStatus: (featureId, status) => {
+    // NOTE: getState() is called outside set() because XState actors are external
+    // side effects that cannot run inside Zustand's synchronous updater. The feature
+    // lookup and actor state restoration use this snapshot, with the actual state
+    // write deferred to the set() call below. This is intentional architecture.
     const state = useRoadmapStore.getState();
     if (!state.roadmap) return;
 

apps/frontend/src/shared/state-machines/roadmap-generation-machine.ts

@@ -86,7 +86,7 @@ export const roadmapGenerationMachine = createMachine(
       }),
       updateProgress: assign({
         progress: ({ event }) =>
-          event.type === 'PROGRESS_UPDATE' ? event.progress : 0,
+          event.type === 'PROGRESS_UPDATE' ? Math.min(100, Math.max(0, event.progress)) : 0,
         message: ({ event }) =>
           event.type === 'PROGRESS_UPDATE' ? event.message : undefined,
       }),
@@ -97,6 +97,7 @@ export const roadmapGenerationMachine = createMachine(
       setError: assign({
         error: ({ event }) =>
           event.type === 'GENERATION_ERROR' ? event.error : undefined,
+        progress: () => 0,
       }),
       resetContext: assign({
         progress: () => 0,

apps/frontend/src/shared/state-machines/roadmap-state-utils.ts

@@ -35,6 +35,10 @@ export type FeatureStateName = typeof FEATURE_STATE_NAMES[number];
  * Generation states where the machine has settled — the generation is
  * complete or has errored. Stale progress events should NOT overwrite
  * these states, as XState is the source of truth.
+ *
+ * NOTE: Exported for future consumer use (e.g., UI components that need to
+ * check if generation is settled before allowing user actions). Currently
+ * unused but intentionally retained as public API for roadmap state checking.
  */
 export const GENERATION_SETTLED_STATES: ReadonlySet<string> = new Set<GenerationStateName>([
   'complete', 'error'
@@ -43,6 +47,10 @@ export const GENERATION_SETTLED_STATES: ReadonlySet<string> = new Set<Generation
 /**
  * Feature states where the machine has settled — the feature is done.
  * Stale task lifecycle events should NOT overwrite this state.
+ *
+ * NOTE: Exported for future consumer use (e.g., UI components that need to
+ * check if feature is settled before allowing drag-and-drop or status changes).
+ * Currently unused but intentionally retained as public API for feature state checking.
  */
 export const FEATURE_SETTLED_STATES: ReadonlySet<string> = new Set<FeatureStateName>([
   'done'