Fix follow-up review findings: data loss bug, dialog nesting, and quality improvements

- Preserve manual competitors in analyze(enabled=False) path (HIGH data loss bug)
- Fix incomplete rollback by restoring full previous competitorAnalysis state
- Reset showAddDialog state when ExistingCompetitorAnalysisDialog reopens
- Add encoding option to writeFileWithRetry for competitor analysis save
- Replace X icon with AlertCircle in error alert for clarity
- Add type="button" to all raw button elements in dialog
- Add warning logs to silent exception handlers in competitor_analyzer.py
- Forward onCompetitorAdded callback through ExistingCompetitorAnalysisDialog

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

apps/backend/runners/roadmap/competitor_analyzer.py

@@ -42,7 +42,10 @@ class CompetitorAnalyzer:
         """
         if not enabled:
             print_status("Competitor analysis not enabled, skipping", "info")
+            manual_competitors = self._get_manual_competitors()
             self._create_disabled_analysis_file()
+            if manual_competitors:
+                self._merge_manual_competitors(manual_competitors)
             return RoadmapPhaseResult(
                 "competitor_analysis", True, [str(self.analysis_file)], [], 0
             )
@@ -133,7 +136,8 @@ class CompetitorAnalyzer:
                 for c in data.get("competitors", [])
                 if isinstance(c, dict) and c.get("source") == "manual"
             ]
-        except (json.JSONDecodeError, OSError):
+        except (json.JSONDecodeError, OSError) as e:
+            print_status(f"Warning: could not read manual competitors: {e}", "warning")
             return []
 
     def _merge_manual_competitors(self, manual_competitors: list[dict]) -> None:
@@ -195,8 +199,11 @@ Output your findings to competitor_analysis.json.
                     "competitor_analysis", True, [str(self.analysis_file)], [], 0
                 )
 
-        except json.JSONDecodeError:
-            pass
+        except json.JSONDecodeError as e:
+            print_status(
+                f"Warning: competitor analysis file is not valid JSON: {e}",
+                "warning",
+            )
 
         return None
 

apps/frontend/src/main/ipc-handlers/roadmap-handlers.ts

@@ -877,7 +877,8 @@ ${(feature.acceptance_criteria || []).map((c: string) => `- [ ] ${c}`).join("\n"
 
           await writeFileWithRetry(
             competitorAnalysisPath,
-            JSON.stringify(serialized, null, 2)
+            JSON.stringify(serialized, null, 2),
+            { encoding: 'utf-8' }
           );
         });
 

apps/frontend/src/renderer/components/AddCompetitorDialog.tsx

@@ -21,7 +21,7 @@
  */
 import { useState, useEffect } from 'react';
 import { useTranslation } from 'react-i18next';
-import { Loader2, X } from 'lucide-react';
+import { Loader2, AlertCircle } from 'lucide-react';
 import {
   Dialog,
   DialogContent,
@@ -142,6 +142,9 @@ export function AddCompetitorDialog({
     setError(null);
 
     try {
+      // Capture pre-add state for complete rollback
+      const previousAnalysis = useRoadmapStore.getState().competitorAnalysis;
+
       // Add competitor to store
       const newCompetitorId = addCompetitor({
         name: name.trim(),
@@ -156,7 +159,7 @@ export function AddCompetitorDialog({
         const result = await window.electronAPI.saveCompetitorAnalysis(projectId, competitorAnalysis);
         if (!result.success) {
           // Rollback store state since save failed
-          useRoadmapStore.getState().removeCompetitor(newCompetitorId);
+          useRoadmapStore.getState().setCompetitorAnalysis(previousAnalysis);
           throw new Error(result.error || t('addCompetitor.failedToAdd'));
         }
       }
@@ -262,7 +265,7 @@ export function AddCompetitorDialog({
           {/* Error */}
           {error && (
             <div className="flex items-start gap-2 rounded-lg bg-destructive/10 border border-destructive/30 p-3 text-sm text-destructive" role="alert">
-              <X className="h-4 w-4 mt-0.5 shrink-0" />
+              <AlertCircle className="h-4 w-4 mt-0.5 shrink-0" />
               <span>{error}</span>
             </div>
           )}

apps/frontend/src/renderer/components/ExistingCompetitorAnalysisDialog.tsx

@@ -1,4 +1,4 @@
-import { useState } from 'react';
+import { useState, useEffect } from 'react';
 import { useTranslation } from 'react-i18next';
 import { Globe, RefreshCw, TrendingUp, CheckCircle, UserPlus } from 'lucide-react';
 import {
@@ -18,6 +18,7 @@ interface ExistingCompetitorAnalysisDialogProps {
   onUseExisting: () => void;
   onRunNew: () => void;
   onSkip: () => void;
+  onCompetitorAdded?: (competitorId: string) => void;
   analysisDate?: Date;
   projectId: string;
 }
@@ -28,12 +29,20 @@ export function ExistingCompetitorAnalysisDialog({
   onUseExisting,
   onRunNew,
   onSkip,
+  onCompetitorAdded,
   analysisDate,
   projectId,
 }: ExistingCompetitorAnalysisDialogProps) {
   const { t, i18n } = useTranslation(['dialogs']);
   const [showAddDialog, setShowAddDialog] = useState(false);
 
+  // Reset child dialog state when this dialog reopens
+  useEffect(() => {
+    if (open) {
+      setShowAddDialog(false);
+    }
+  }, [open]);
+
   const handleUseExisting = () => {
     onUseExisting();
     onOpenChange(false);
@@ -75,6 +84,7 @@ export function ExistingCompetitorAnalysisDialog({
           <div className="py-4 space-y-3">
             {/* Option 1: Use existing (recommended) */}
             <button
+              type="button"
               onClick={handleUseExisting}
               className="w-full rounded-lg bg-primary/10 border border-primary/30 p-4 text-left hover:bg-primary/20 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2"
             >
@@ -94,6 +104,7 @@ export function ExistingCompetitorAnalysisDialog({
 
             {/* Option 2: Run new analysis */}
             <button
+              type="button"
               onClick={handleRunNew}
               className="w-full rounded-lg bg-muted/50 border border-border p-4 text-left hover:bg-muted transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2"
             >
@@ -112,6 +123,7 @@ export function ExistingCompetitorAnalysisDialog({
 
             {/* Option 3: Add known competitors */}
             <button
+              type="button"
               onClick={() => setShowAddDialog(true)}
               className="w-full rounded-lg bg-muted/50 border border-border p-4 text-left hover:bg-muted transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2"
             >
@@ -130,6 +142,7 @@ export function ExistingCompetitorAnalysisDialog({
 
             {/* Option 4: Skip */}
             <button
+              type="button"
               onClick={handleSkip}
               className="w-full rounded-lg bg-muted/30 border border-border/50 p-4 text-left hover:bg-muted/50 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2"
             >
@@ -158,6 +171,7 @@ export function ExistingCompetitorAnalysisDialog({
       <AddCompetitorDialog
         open={showAddDialog}
         onOpenChange={setShowAddDialog}
+        onCompetitorAdded={onCompetitorAdded}
         projectId={projectId}
       />
     </>