fix: address PR feedback for delete insight feature

- Fix AlertDialogAction closing before async delete completes by using
  asChild with Button and event.preventDefault()
- Add delete button to SourceInsightDialog header for consistent UX
- Add confirmation dialog in SourceInsightDialog
- Pass onDelete callback from SourceDetailContent to SourceInsightDialog

Addresses PR #334 feedback from lfnovo and cubic-dev-ai.

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

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

frontend/src/components/source/SourceDetailContent.tsx

@@ -164,7 +164,8 @@ export function SourceDetailContent({
     }
   }
 
-  const handleDeleteInsight = async () => {
+  const handleDeleteInsight = async (e?: React.MouseEvent) => {
+    e?.preventDefault()
     if (!insightToDelete) return
 
     try {
@@ -780,6 +781,12 @@ export function SourceDetailContent({
           }
         }}
         insight={selectedInsight ?? undefined}
+        onDelete={async (insightId) => {
+          await insightsApi.delete(insightId)
+          toast.success('Insight deleted successfully')
+          setSelectedInsight(null)
+          await fetchInsights()
+        }}
       />
 
       <AlertDialog open={!!insightToDelete} onOpenChange={() => setInsightToDelete(null)}>
@@ -792,12 +799,14 @@ export function SourceDetailContent({
           </AlertDialogHeader>
           <AlertDialogFooter>
             <AlertDialogCancel disabled={deletingInsight}>Cancel</AlertDialogCancel>
-            <AlertDialogAction
+            <AlertDialogAction asChild>
-              onClick={handleDeleteInsight}
+              <Button
-              disabled={deletingInsight}
+                onClick={handleDeleteInsight}
-              className="bg-destructive text-destructive-foreground hover:bg-destructive/90"
+                disabled={deletingInsight}
-            >
+                variant="destructive"
-              {deletingInsight ? 'Deleting...' : 'Delete'}
+              >
+                {deletingInsight ? 'Deleting...' : 'Delete'}
+              </Button>
             </AlertDialogAction>
           </AlertDialogFooter>
         </AlertDialogContent>

frontend/src/components/source/SourceInsightDialog.tsx

@@ -1,7 +1,10 @@
 'use client'
 
-import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog'
+import { useState, useEffect } from 'react'
+import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogFooter } from '@/components/ui/dialog'
 import { Badge } from '@/components/ui/badge'
+import { Button } from '@/components/ui/button'
+import { Trash2 } from 'lucide-react'
 import ReactMarkdown from 'react-markdown'
 import remarkGfm from 'remark-gfm'
 import { useInsight } from '@/lib/hooks/use-insights'
@@ -15,9 +18,13 @@ interface SourceInsightDialogProps {
     content?: string
     created?: string
   }
+  onDelete?: (insightId: string) => Promise<void>
 }
 
-export function SourceInsightDialog({ open, onOpenChange, insight }: SourceInsightDialogProps) {
+export function SourceInsightDialog({ open, onOpenChange, insight, onDelete }: SourceInsightDialogProps) {
+  const [showDeleteConfirm, setShowDeleteConfirm] = useState(false)
+  const [isDeleting, setIsDeleting] = useState(false)
+
   // Ensure insight ID has 'source_insight:' prefix for API calls
   const insightIdWithPrefix = insight?.id
     ? (insight.id.includes(':') ? insight.id : `source_insight:${insight.id}`)
@@ -28,49 +35,108 @@ export function SourceInsightDialog({ open, onOpenChange, insight }: SourceInsig
   // Use fetched data if available, otherwise fall back to passed-in insight
   const displayInsight = fetchedInsight ?? insight
 
+  // Reset delete state when dialog closes
+  useEffect(() => {
+    if (!open) {
+      setShowDeleteConfirm(false)
+      setIsDeleting(false)
+    }
+  }, [open])
+
+  const handleDelete = async () => {
+    if (!insight?.id || !onDelete) return
+
+    try {
+      setIsDeleting(true)
+      await onDelete(insight.id)
+      // Parent's onDelete callback handles closing the dialog via setSelectedInsight(null)
+    } catch {
+      // Only reset state if delete failed - dialog stays open
+      setShowDeleteConfirm(false)
+      setIsDeleting(false)
+    }
+  }
+
   return (
     <Dialog open={open} onOpenChange={onOpenChange}>
       <DialogContent className="sm:max-w-3xl max-h-[90vh] flex flex-col">
         <DialogHeader>
           <DialogTitle className="flex items-center justify-between gap-2">
             <span>Source Insight</span>
-            {displayInsight?.insight_type && (
+            <div className="flex items-center gap-2">
-              <Badge variant="outline" className="text-xs uppercase">
+              {displayInsight?.insight_type && (
-                {displayInsight.insight_type}
+                <Badge variant="outline" className="text-xs uppercase">
-              </Badge>
+                  {displayInsight.insight_type}
-            )}
+                </Badge>
+              )}
+              {onDelete && insight?.id && !showDeleteConfirm && (
+                <Button
+                  size="sm"
+                  variant="outline"
+                  onClick={() => setShowDeleteConfirm(true)}
+                  className="text-destructive hover:text-destructive"
+                >
+                  <Trash2 className="h-4 w-4" />
+                </Button>
+              )}
+            </div>
           </DialogTitle>
         </DialogHeader>
 
-        <div className="flex-1 overflow-y-auto min-h-0">
+        {showDeleteConfirm ? (
-          {isLoading ? (
+          <div className="flex flex-col items-center justify-center py-8 gap-4">
-            <div className="flex items-center justify-center py-10">
+            <p className="text-center text-muted-foreground">
-              <span className="text-sm text-muted-foreground">Loading insight…</span>
+              Are you sure you want to delete this insight?<br />
-            </div>
+              <span className="text-sm">This action cannot be undone.</span>
-          ) : displayInsight ? (
+            </p>
-            <div className="prose prose-sm prose-neutral dark:prose-invert max-w-none">
+            <div className="flex gap-2">
-              <ReactMarkdown
+              <Button
-                remarkPlugins={[remarkGfm]}
+                variant="outline"
-                components={{
+                onClick={() => setShowDeleteConfirm(false)}
-                  table: ({ children }) => (
+                disabled={isDeleting}
-                    <div className="my-4 overflow-x-auto">
-                      <table className="min-w-full border-collapse border border-border">{children}</table>
-                    </div>
-                  ),
-                  thead: ({ children }) => <thead className="bg-muted">{children}</thead>,
-                  tbody: ({ children }) => <tbody>{children}</tbody>,
-                  tr: ({ children }) => <tr className="border-b border-border">{children}</tr>,
-                  th: ({ children }) => <th className="border border-border px-3 py-2 text-left font-semibold">{children}</th>,
-                  td: ({ children }) => <td className="border border-border px-3 py-2">{children}</td>,
-                }}
               >
-                {displayInsight.content}
+                Cancel
-              </ReactMarkdown>
+              </Button>
+              <Button
+                variant="destructive"
+                onClick={handleDelete}
+                disabled={isDeleting}
+              >
+                {isDeleting ? 'Deleting...' : 'Delete'}
+              </Button>
             </div>
-          ) : (
+          </div>
-            <p className="text-sm text-muted-foreground">No insight selected.</p>
+        ) : (
-          )}
+          <div className="flex-1 overflow-y-auto min-h-0">
-        </div>
+            {isLoading ? (
+              <div className="flex items-center justify-center py-10">
+                <span className="text-sm text-muted-foreground">Loading insight…</span>
+              </div>
+            ) : displayInsight ? (
+              <div className="prose prose-sm prose-neutral dark:prose-invert max-w-none">
+                <ReactMarkdown
+                  remarkPlugins={[remarkGfm]}
+                  components={{
+                    table: ({ children }) => (
+                      <div className="my-4 overflow-x-auto">
+                        <table className="min-w-full border-collapse border border-border">{children}</table>
+                      </div>
+                    ),
+                    thead: ({ children }) => <thead className="bg-muted">{children}</thead>,
+                    tbody: ({ children }) => <tbody>{children}</tbody>,
+                    tr: ({ children }) => <tr className="border-b border-border">{children}</tr>,
+                    th: ({ children }) => <th className="border border-border px-3 py-2 text-left font-semibold">{children}</th>,
+                    td: ({ children }) => <td className="border border-border px-3 py-2">{children}</td>,
+                  }}
+                >
+                  {displayInsight.content}
+                </ReactMarkdown>
+              </div>
+            ) : (
+              <p className="text-sm text-muted-foreground">No insight selected.</p>
+            )}
+          </div>
+        )}
       </DialogContent>
     </Dialog>
   )