fase(20): visual regression refactor

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

frontend/src/App.tsx

@@ -24,10 +24,7 @@ import { AppearanceSection } from '@/pages/settings/AppearanceSection'
 import { LicenseSection } from '@/pages/settings/LicenseSection'
 import { SchedulesSection } from '@/pages/settings/SchedulesSection'
 import { Reports } from '@/pages/Reports'
-
-function VisualReview() {
-  return <div className="text-muted-foreground p-4">Visual Review — Coming in Phase 20</div>
-}
+import { VisualReview } from '@/pages/VisualReview'
 
 export default function App() {
   return (

frontend/src/pages/VisualReview.tsx

@@ -1,279 +1,288 @@
-import { useState, useEffect, useCallback } from 'react';
-import { Link } from 'react-router-dom';
-import { api } from '../hooks/useApi';
-import type { VisualComparison, ComparisonStatus } from '../types';
+import { useState, useCallback } from 'react';
+import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
+import { Badge } from '@/components/ui/badge';
+import { Button } from '@/components/ui/button';
+import { Card, CardContent, CardHeader } from '@/components/ui/card';
+import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogFooter } from '@/components/ui/dialog';
+import { Skeleton } from '@/components/ui/skeleton';
+import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select';
+import { api } from '@/hooks/useApi';
+import type { VisualComparison, ComparisonStatus } from '@/types';
 
-const STATUS_LABEL: Record<ComparisonStatus, string> = {
+const STATUS_LABEL: Record<ComparisonStatus | 'all', string> = {
+  all: 'All',
   new_state: 'New State',
   failed: 'Regression',
   passed: 'Passed',
   pending: 'Pending',
 };
 
-const STATUS_COLORS: Record<ComparisonStatus, string> = {
-  new_state: 'bg-blue-900/50 text-blue-300 border-blue-700',
-  failed: 'bg-red-900/50 text-red-300 border-red-700',
-  passed: 'bg-green-900/50 text-green-300 border-green-700',
-  pending: 'bg-gray-700 text-gray-300 border-gray-600',
+const STATUS_VARIANT: Record<ComparisonStatus, 'default' | 'destructive' | 'secondary' | 'outline'> = {
+  new_state: 'default',
+  failed: 'destructive',
+  passed: 'secondary',
+  pending: 'outline',
 };
 
-function screenshotUrl(path: string | null): string | null {
-  if (!path) return null;
-  // Backend serves screenshots from /api/screenshots/<filename>
-  const parts = path.replace(/\\/g, '/').split('/');
+function screenshotUrl(filePath: string | null): string | null {
+  if (!filePath) return null;
+  const parts = filePath.replace(/\\/g, '/').split('/');
   return `/api/screenshots/${parts[parts.length - 1]}`;
 }
 
-interface ComparisonModalProps {
+interface ComparisonDialogProps {
   comparison: VisualComparison;
-  onApprove: () => void;
-  onReject: () => void;
   onClose: () => void;
+  onApproved: () => void;
+  onRejected: () => void;
 }
 
-function ComparisonModal({ comparison, onApprove, onReject, onClose }: ComparisonModalProps) {
-  const [acting, setActing] = useState<'approving' | 'rejecting' | null>(null);
+function ComparisonDialog({ comparison, onClose, onApproved, onRejected }: ComparisonDialogProps) {
+  const approveMutation = useMutation({
+    mutationFn: () => api.approveBaseline(comparison.id),
+    onSuccess: onApproved,
+  });
 
-  async function handleApprove() {
-    setActing('approving');
-    try { await api.approveBaseline(comparison.id); onApprove(); }
-    finally { setActing(null); }
-  }
-
-  async function handleReject() {
-    setActing('rejecting');
-    try { await api.rejectBaseline(comparison.id); onReject(); }
-    finally { setActing(null); }
-  }
+  const rejectMutation = useMutation({
+    mutationFn: () => api.rejectBaseline(comparison.id),
+    onSuccess: onRejected,
+  });
 
   const currentUrl = screenshotUrl(comparison.current_screenshot_path);
   const diffUrl = screenshotUrl(comparison.diff_screenshot_path);
 
   return (
-    <div
-      className="fixed inset-0 bg-black/70 flex items-start justify-center z-50 overflow-y-auto py-8"
-      onClick={onClose}
-    >
-      <div
-        className="bg-gray-800 rounded-lg w-full max-w-5xl mx-4 shadow-2xl"
-        onClick={(e) => e.stopPropagation()}
-      >
-        <div className="flex items-center justify-between px-6 py-4 border-b border-gray-700">
-          <div>
-            <h3 className="text-base font-semibold text-white">Visual Review</h3>
-            <p className="text-xs text-gray-400 mt-0.5">
-              State: <code className="bg-gray-700 px-1 rounded">{comparison.state_id.slice(0, 12)}…</code>
-              {comparison.diff_percent !== null && (
-                <span className="ml-3">Diff: <strong className="text-red-400">{comparison.diff_percent.toFixed(2)}%</strong></span>
-              )}
-            </p>
-          </div>
-          <button onClick={onClose} className="text-gray-400 hover:text-white text-xl leading-none">&times;</button>
-        </div>
+    <Dialog open onOpenChange={(open) => { if (!open) onClose(); }}>
+      <DialogContent className="max-w-5xl w-full">
+        <DialogHeader>
+          <DialogTitle className="flex items-center gap-3">
+            Visual Review
+            <Badge variant={STATUS_VARIANT[comparison.status]}>
+              {STATUS_LABEL[comparison.status]}
+            </Badge>
+          </DialogTitle>
+          <p className="text-sm text-muted-foreground">
+            State: <code className="bg-muted px-1 rounded text-xs">{comparison.state_id.slice(0, 16)}…</code>
+            {comparison.diff_percent !== null && (
+              <span className="ml-3 text-destructive font-medium">
+                {comparison.diff_percent.toFixed(2)}% diff
+              </span>
+            )}
+          </p>
+        </DialogHeader>
 
-        <div className="p-6 grid grid-cols-1 md:grid-cols-3 gap-4">
-          {/* Baseline */}
+        <div className="grid grid-cols-1 md:grid-cols-3 gap-4 py-4">
           <div className="space-y-2">
-            <p className="text-xs text-gray-400 font-medium uppercase tracking-wide">Baseline</p>
+            <p className="text-xs font-medium uppercase tracking-wide text-muted-foreground">Baseline</p>
             {comparison.baseline_id ? (
-              <div className="bg-gray-900 rounded overflow-hidden">
-                <img src={`/api/visual/baseline-screenshot/${comparison.baseline_id}`} alt="Baseline" className="w-full" />
+              <div className="bg-muted rounded overflow-hidden">
+                <img
+                  src={`/api/visual/baseline-screenshot/${comparison.baseline_id}`}
+                  alt="Baseline"
+                  className="w-full"
+                />
               </div>
             ) : (
-              <div className="bg-gray-900 rounded p-8 text-center text-gray-500 text-sm">No baseline</div>
+              <div className="bg-muted rounded p-8 text-center text-muted-foreground text-sm">
+                No baseline
+              </div>
             )}
           </div>
 
-          {/* Current */}
           <div className="space-y-2">
-            <p className="text-xs text-gray-400 font-medium uppercase tracking-wide">Current</p>
+            <p className="text-xs font-medium uppercase tracking-wide text-muted-foreground">Current</p>
             {currentUrl ? (
-              <div className="bg-gray-900 rounded overflow-hidden">
+              <div className="bg-muted rounded overflow-hidden">
                 <img src={currentUrl} alt="Current screenshot" className="w-full" />
               </div>
             ) : (
-              <div className="bg-gray-900 rounded p-8 text-center text-gray-500 text-sm">No screenshot</div>
+              <div className="bg-muted rounded p-8 text-center text-muted-foreground text-sm">
+                No screenshot
+              </div>
             )}
           </div>
 
-          {/* Diff */}
           <div className="space-y-2">
-            <p className="text-xs text-gray-400 font-medium uppercase tracking-wide">Diff</p>
+            <p className="text-xs font-medium uppercase tracking-wide text-muted-foreground">Diff</p>
             {diffUrl ? (
-              <div className="bg-gray-900 rounded overflow-hidden">
+              <div className="bg-muted rounded overflow-hidden">
                 <img src={diffUrl} alt="Diff image" className="w-full" />
               </div>
             ) : (
-              <div className="bg-gray-900 rounded p-8 text-center text-gray-500 text-sm">No diff</div>
+              <div className="bg-muted rounded p-8 text-center text-muted-foreground text-sm">
+                No diff
+              </div>
             )}
           </div>
         </div>
 
-        <div className="px-6 pb-6 flex gap-3">
+        <DialogFooter className="gap-2">
           {comparison.status !== 'passed' && (
-            <button
-              onClick={handleApprove}
-              disabled={!!acting}
-              className="bg-green-600 hover:bg-green-500 disabled:opacity-50 text-white text-sm font-medium px-5 py-2 rounded transition-colors"
+            <Button
+              variant="default"
+              onClick={() => approveMutation.mutate()}
+              disabled={approveMutation.isPending || rejectMutation.isPending}
             >
-              {acting === 'approving' ? 'Approving…' : 'Approve as Baseline'}
-            </button>
+              {approveMutation.isPending ? 'Approving…' : 'Approve as Baseline'}
+            </Button>
           )}
           {comparison.status === 'failed' && (
-            <button
-              onClick={handleReject}
-              disabled={!!acting}
-              className="bg-red-700 hover:bg-red-600 disabled:opacity-50 text-white text-sm font-medium px-5 py-2 rounded transition-colors"
+            <Button
+              variant="destructive"
+              onClick={() => rejectMutation.mutate()}
+              disabled={approveMutation.isPending || rejectMutation.isPending}
             >
-              {acting === 'rejecting' ? 'Rejecting…' : 'Mark as Rejected'}
-            </button>
+              {rejectMutation.isPending ? 'Rejecting…' : 'Mark as Rejected'}
+            </Button>
           )}
-          <button
-            onClick={onClose}
-            className="bg-gray-700 hover:bg-gray-600 text-gray-200 text-sm font-medium px-5 py-2 rounded transition-colors"
-          >
+          <Button variant="outline" onClick={onClose}>
             Close
-          </button>
-        </div>
-      </div>
-    </div>
+          </Button>
+        </DialogFooter>
+      </DialogContent>
+    </Dialog>
   );
 }
 
-const ALL_STATUSES: Array<ComparisonStatus | 'all'> = ['all', 'new_state', 'failed', 'passed', 'pending'];
-
 export function VisualReview() {
-  const [comparisons, setComparisons] = useState<VisualComparison[]>([]);
   const [statusFilter, setStatusFilter] = useState<ComparisonStatus | 'all'>('all');
-  const [loading, setLoading] = useState(true);
   const [selected, setSelected] = useState<VisualComparison | null>(null);
-  const [approvingAll, setApprovingAll] = useState(false);
-  const [bulkResult, setBulkResult] = useState<string | null>(null);
+  const queryClient = useQueryClient();
 
-  const load = useCallback(() => {
-    setLoading(true);
-    api
-      .getVisualComparisons(statusFilter !== 'all' ? { status: statusFilter } : undefined)
-      .then(setComparisons)
-      .catch(() => setComparisons([]))
-      .finally(() => setLoading(false));
-  }, [statusFilter]);
+  const { data: comparisons = [], isLoading } = useQuery({
+    queryKey: ['visual-comparisons', statusFilter],
+    queryFn: () =>
+      api.getVisualComparisons(statusFilter !== 'all' ? { status: statusFilter } : undefined),
+  });
 
-  useEffect(() => { load(); }, [load]);
+  const approveAllMutation = useMutation({
+    mutationFn: () => api.approveAllBaselines(),
+    onSuccess: () => void queryClient.invalidateQueries({ queryKey: ['visual-comparisons'] }),
+  });
 
-  async function handleApproveAll() {
-    if (!confirm('Approve all new_state comparisons as baselines?')) return;
-    setApprovingAll(true);
-    setBulkResult(null);
-    try {
-      const res = await api.approveAllBaselines();
-      setBulkResult(`Approved ${res.approved} comparison(s) as baselines.`);
-      load();
-    } catch (err) {
-      setBulkResult(`Error: ${err instanceof Error ? err.message : 'unknown'}`);
-    } finally {
-      setApprovingAll(false);
-    }
-  }
+  const handleRefresh = useCallback(() => {
+    void queryClient.invalidateQueries({ queryKey: ['visual-comparisons'] });
+  }, [queryClient]);
 
   const newStateCount = comparisons.filter((c) => c.status === 'new_state').length;
   const failedCount = comparisons.filter((c) => c.status === 'failed').length;
 
   return (
-    <div className="max-w-6xl mx-auto px-4 py-8 space-y-6">
-      <header className="flex items-center justify-between">
+    <div className="p-6 space-y-6">
+      <div className="flex items-start justify-between">
         <div>
-          <Link to="/" className="text-blue-400 text-sm hover:underline">← Dashboard</Link>
-          <h1 className="text-2xl font-bold text-white mt-1">Visual Regression Review</h1>
-          <p className="text-gray-400 text-sm mt-1">
-            {failedCount > 0 && <span className="text-red-400 font-medium mr-3">{failedCount} regression{failedCount > 1 ? 's' : ''}</span>}
-            {newStateCount > 0 && <span className="text-blue-400 font-medium">{newStateCount} new state{newStateCount > 1 ? 's' : ''}</span>}
+          <h1 className="text-2xl font-bold tracking-tight">Visual Regression Review</h1>
+          <p className="text-muted-foreground text-sm mt-1">
+            {failedCount > 0 && (
+              <span className="text-destructive font-medium mr-3">
+                {failedCount} regression{failedCount !== 1 ? 's' : ''}
+              </span>
+            )}
+            {newStateCount > 0 && (
+              <span className="text-primary font-medium">
+                {newStateCount} new state{newStateCount !== 1 ? 's' : ''}
+              </span>
+            )}
+            {failedCount === 0 && newStateCount === 0 && comparisons.length > 0 && (
+              <span>All comparisons reviewed</span>
+            )}
           </p>
         </div>
         {newStateCount > 0 && (
-          <button
-            onClick={handleApproveAll}
-            disabled={approvingAll}
-            className="bg-green-600 hover:bg-green-500 disabled:opacity-50 text-white text-sm font-medium px-4 py-2 rounded transition-colors"
+          <Button
+            onClick={() => approveAllMutation.mutate()}
+            disabled={approveAllMutation.isPending}
           >
-            {approvingAll ? 'Approving…' : `Approve All New (${newStateCount})`}
-          </button>
+            {approveAllMutation.isPending
+              ? 'Approving…'
+              : `Approve All New (${newStateCount})`}
+          </Button>
         )}
-      </header>
-
-      {bulkResult && (
-        <div className="bg-gray-800 rounded px-4 py-3 text-sm text-gray-300">
-          {bulkResult}
-        </div>
-      )}
-
-      {/* Status filter */}
-      <div className="flex gap-2">
-        {ALL_STATUSES.map((s) => (
-          <button
-            key={s}
-            onClick={() => setStatusFilter(s)}
-            className={`text-xs px-3 py-1.5 rounded border transition-colors ${
-              statusFilter === s
-                ? 'bg-gray-600 border-gray-500 text-white'
-                : 'bg-transparent border-gray-700 text-gray-400 hover:text-gray-200'
-            }`}
-          >
-            {s === 'all' ? 'All' : STATUS_LABEL[s]}
-          </button>
-        ))}
       </div>
 
-      {loading ? (
-        <p className="text-gray-400 text-sm">Loading comparisons…</p>
-      ) : comparisons.length === 0 ? (
-        <div className="bg-gray-800 rounded-lg p-12 text-center text-gray-500 text-sm">
-          No visual comparisons found. Run an exploration with visual regression enabled.
+      {/* Status filter */}
+      <div className="flex items-center gap-3">
+        <span className="text-sm text-muted-foreground">Filter:</span>
+        <Select
+          value={statusFilter}
+          onValueChange={(v) => setStatusFilter(v as ComparisonStatus | 'all')}
+        >
+          <SelectTrigger className="w-40">
+            <SelectValue />
+          </SelectTrigger>
+          <SelectContent>
+            <SelectItem value="all">All</SelectItem>
+            <SelectItem value="new_state">New State</SelectItem>
+            <SelectItem value="failed">Regression</SelectItem>
+            <SelectItem value="passed">Passed</SelectItem>
+            <SelectItem value="pending">Pending</SelectItem>
+          </SelectContent>
+        </Select>
+      </div>
+
+      {/* Grid */}
+      {isLoading ? (
+        <div className="grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-4 gap-4">
+          {Array.from({ length: 8 }).map((_, i) => (
+            <Skeleton key={i} className="aspect-video rounded-lg" />
+          ))}
         </div>
+      ) : comparisons.length === 0 ? (
+        <Card>
+          <CardContent className="py-16 text-center text-muted-foreground">
+            No visual comparisons found. Run an exploration with visual regression enabled.
+          </CardContent>
+        </Card>
       ) : (
         <div className="grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-4 gap-4">
           {comparisons.map((cmp) => {
             const imgUrl = screenshotUrl(cmp.current_screenshot_path);
             return (
-              <button
+              <Card
                 key={cmp.id}
+                className="overflow-hidden cursor-pointer hover:ring-2 hover:ring-ring transition-all"
                 onClick={() => setSelected(cmp)}
-                className="bg-gray-800 rounded-lg overflow-hidden hover:ring-2 hover:ring-blue-500 transition-all text-left"
               >
-                {imgUrl ? (
-                  <div className="bg-gray-900 aspect-video overflow-hidden">
-                    <img src={imgUrl} alt="Screenshot" className="w-full object-cover object-top" />
-                  </div>
-                ) : (
-                  <div className="bg-gray-900 aspect-video flex items-center justify-center text-gray-600 text-xs">
-                    No image
-                  </div>
-                )}
-                <div className="p-3 space-y-1">
-                  <span
-                    className={`inline-block text-xs px-2 py-0.5 rounded border ${STATUS_COLORS[cmp.status]}`}
-                  >
+                <CardHeader className="p-0">
+                  {imgUrl ? (
+                    <div className="bg-muted aspect-video overflow-hidden">
+                      <img
+                        src={imgUrl}
+                        alt="Screenshot"
+                        className="w-full object-cover object-top"
+                      />
+                    </div>
+                  ) : (
+                    <div className="bg-muted aspect-video flex items-center justify-center text-muted-foreground text-xs">
+                      No image
+                    </div>
+                  )}
+                </CardHeader>
+                <CardContent className="p-3 space-y-1">
+                  <Badge variant={STATUS_VARIANT[cmp.status]}>
                     {STATUS_LABEL[cmp.status]}
-                  </span>
-                  <p className="text-gray-400 text-xs truncate">
+                  </Badge>
+                  <p className="text-muted-foreground text-xs truncate">
                     {cmp.state_id.slice(0, 16)}…
                   </p>
                   {cmp.diff_percent !== null && (
-                    <p className="text-red-400 text-xs font-medium">{cmp.diff_percent.toFixed(2)}% diff</p>
+                    <p className="text-destructive text-xs font-medium">
+                      {cmp.diff_percent.toFixed(2)}% diff
+                    </p>
                   )}
-                </div>
-              </button>
+                </CardContent>
+              </Card>
             );
           })}
         </div>
       )}
 
       {selected && (
-        <ComparisonModal
+        <ComparisonDialog
           comparison={selected}
-          onApprove={() => { setSelected(null); load(); }}
-          onReject={() => { setSelected(null); load(); }}
           onClose={() => setSelected(null)}
+          onApproved={() => { setSelected(null); handleRefresh(); }}
+          onRejected={() => { setSelected(null); handleRefresh(); }}
         />
       )}
     </div>