From db82eb4fedfeb69de26aeb3a7a7ded4e5f7bbd0b Mon Sep 17 00:00:00 2001 From: "Leivur R. Djurhuus" Date: Sat, 14 Mar 2026 15:32:41 -0500 Subject: [PATCH] refactor: simplify feedback from 4-level severity to action item / info callout Replace FeedbackSeverity enum (Critical/Major/Minor/Suggestion) with a simple isActionItem boolean. Annotations default to action items (things the artist must fix). Any item can be toggled to an info callout (context that doesn't need action). Progress bar and carry-forward only count action items. Screenshot paste limited to 5MB with user notification. Co-Authored-By: Claude Opus 4.6 (1M context) --- ROADMAP.md | 29 ++- prisma/schema.prisma | 12 +- .../api/stages/[stageId]/feedback/route.ts | 8 +- src/components/review/feedback-checklist.tsx | 172 +++++++++--------- src/components/review/feedback-item-card.tsx | 101 +++++----- .../review/feedback-progress-bar.tsx | 9 +- src/components/stages/feedback-indicator.tsx | 21 +-- src/hooks/use-annotation-state.ts | 8 + src/hooks/use-feedback.ts | 4 +- src/lib/services/feedback-service.ts | 54 +++--- src/lib/validators/feedback.ts | 11 +- 11 files changed, 212 insertions(+), 217 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 7333957..042abd3 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -247,7 +247,7 @@ enum AnnotationType { RECTANGLE ELLIPSE ARROW FREEHAND TEXT PIN SCREENSHOT --- -#### A4 — Revision History Timeline `[ ]` +#### A4 — Revision History Timeline `[x]` A collapsible sidebar panel in the review page showing the full version history for a deliverable stage. The connective tissue between annotations, comparison, and feedback — provides the longitudinal view across all rounds. @@ -277,28 +277,26 @@ A collapsible sidebar panel in the review page showing the full version history --- -#### A5 — Feedback Checklist (Artist Action Items) `[ ]` +#### A5 — Feedback Checklist (Artist Action Items) `[x]` Every annotation and actionable comment becomes a structured to-do item on a checklist for the assigned artist. Closes the feedback-to-fix loop. **The feedback loop:** 1. Reviewer draws annotation or posts actionable comment 2. System auto-creates a FeedbackItem linked to the annotation/comment -3. Artist sees checklist — organized by severity, with direct links to annotation on the image +3. Artist sees checklist — action items with direct links to annotations on the image 4. Artist works through items — checks each off with optional resolution note -5. Artist submits new revision — unchecked items carry forward with a warning +5. Artist submits new revision — unchecked action items carry forward with a warning 6. Reviewer verifies — can confirm resolution or reopen -**Where the checklist appears (3 locations):** -1. **Review page — Feedback Panel** (primary): full checklist with severity indicators, thumbnail crops of annotated regions, resolve/reopen actions, progress bar -2. **My Work page** — feedback badge per assignment ("5 open items"), expandable inline checklist, deep-link to review page -3. **Stage card on deliverable page** — compact badge ("4/7 resolved"), color-coded by severity +**Two item types (simplified from 4-level severity):** +- **Action Item** (default) — something the artist needs to fix. Has checkbox, can be resolved/verified. +- **Info Callout** — context or reference that doesn't require action (e.g., "FYI the client prefers warmer tones"). No checkbox. Can be toggled from action item and vice versa. -**Severity levels:** -- **Critical** — must fix, blocks approval -- **Major** — should fix, significant quality issue -- **Minor** — nice to fix, small quality issue -- **Suggestion** — optional improvement +**Where the checklist appears (3 locations):** +1. **Review page — Feedback Panel** (primary): full checklist with action items first, then info callouts. Progress bar counts only action items. Filter by type and status. +2. **My Work page** — feedback badge per assignment ("5 open items") +3. **Stage card on deliverable page** — compact badge ("4/7 resolved") for action items **New data model:** ```prisma @@ -313,7 +311,7 @@ model FeedbackItem { commentId String? comment Comment? @relation(...) summary String - severity FeedbackSeverity @default(MAJOR) + isActionItem Boolean @default(true) status FeedbackStatus @default(OPEN) sortOrder Int @default(0) assignedToId String? @@ -329,8 +327,7 @@ model FeedbackItem { @@map("feedback_items") } -enum FeedbackSeverity { CRITICAL MAJOR MINOR SUGGESTION } -enum FeedbackStatus { OPEN IN_PROGRESS RESOLVED VERIFIED REOPENED } +enum FeedbackStatus { OPEN IN_PROGRESS RESOLVED VERIFIED REOPENED } ``` **Key files:** diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 295d045..e2a1c72 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -774,8 +774,7 @@ model FeedbackItem { commentId String? comment Comment? @relation(fields: [commentId], references: [id], onDelete: SetNull) summary String - isActionItem Boolean @default(true) - severity FeedbackSeverity @default(MAJOR) + isActionItem Boolean @default(true) // true = action item (must fix), false = info callout status FeedbackStatus @default(OPEN) sortOrder Int @default(0) assignedToId String? @@ -802,9 +801,6 @@ model FeedbackItem { @@map("feedback_items") } -enum FeedbackSeverity { - CRITICAL - MAJOR - MINOR - SUGGESTION -} +// FeedbackSeverity removed — replaced by isActionItem boolean +// Action items = things the artist must fix (default for annotations) +// Info callouts = context/reference that doesn't need action diff --git a/src/app/api/stages/[stageId]/feedback/route.ts b/src/app/api/stages/[stageId]/feedback/route.ts index 624f668..2508411 100644 --- a/src/app/api/stages/[stageId]/feedback/route.ts +++ b/src/app/api/stages/[stageId]/feedback/route.ts @@ -10,7 +10,7 @@ import { type Params = { params: Promise<{ stageId: string }> }; // GET /api/stages/:stageId/feedback -// Query params: ?revisionId=&status=&severity=&summary=true +// Query params: ?revisionId=&status=&isActionItem=true|false&summary=true export async function GET(request: Request, { params }: Params) { const { error } = await getAuthSession(); if (error) return error; @@ -20,7 +20,9 @@ export async function GET(request: Request, { params }: Params) { const url = new URL(request.url); const revisionId = url.searchParams.get("revisionId") ?? undefined; const status = url.searchParams.get("status") ?? undefined; - const severity = url.searchParams.get("severity") ?? undefined; + const isActionItemParam = url.searchParams.get("isActionItem"); + const isActionItem = + isActionItemParam === "true" ? true : isActionItemParam === "false" ? false : undefined; const summaryOnly = url.searchParams.get("summary") === "true"; if (summaryOnly) { @@ -31,7 +33,7 @@ export async function GET(request: Request, { params }: Params) { const items = await listFeedbackItems(stageId, { revisionId, status, - severity, + isActionItem, }); return NextResponse.json(items); } catch (e) { diff --git a/src/components/review/feedback-checklist.tsx b/src/components/review/feedback-checklist.tsx index aecc59a..8cd7bf6 100644 --- a/src/components/review/feedback-checklist.tsx +++ b/src/components/review/feedback-checklist.tsx @@ -5,22 +5,14 @@ import { ChevronDown, ChevronUp, ClipboardList, - Filter, - Plus, } from "lucide-react"; import { Badge } from "@/components/ui/badge"; -import { Button } from "@/components/ui/button"; -import { Separator } from "@/components/ui/separator"; -import { - Popover, - PopoverContent, - PopoverTrigger, -} from "@/components/ui/popover"; import { TooltipProvider } from "@/components/ui/tooltip"; import { toast } from "sonner"; import { cn } from "@/lib/utils"; import { useFeedbackItems, + useUpdateFeedback, useResolveFeedback, useVerifyFeedback, useReopenFeedback, @@ -40,11 +32,9 @@ interface FeedbackChecklistProps { }) => void; } -type SeverityFilter = "ALL" | "CRITICAL" | "MAJOR" | "MINOR" | "SUGGESTION"; +type TypeFilter = "ALL" | "ACTION" | "INFO"; type StatusFilter = "ALL" | "OPEN" | "RESOLVED"; -const SEVERITY_ORDER = ["CRITICAL", "MAJOR", "MINOR", "SUGGESTION"] as const; - export function FeedbackChecklist({ stageId, revisionId, @@ -52,16 +42,18 @@ export function FeedbackChecklist({ onAnnotationClick, }: FeedbackChecklistProps) { const [collapsed, setCollapsed] = useState(false); - const [severityFilter, setSeverityFilter] = useState("ALL"); + const [typeFilter, setTypeFilter] = useState("ALL"); const [statusFilter, setStatusFilter] = useState("ALL"); const { data: items = [], isLoading } = useFeedbackItems(stageId, revisionId); + const updateMutation = useUpdateFeedback(stageId); const resolveMutation = useResolveFeedback(stageId); const verifyMutation = useVerifyFeedback(stageId); const reopenMutation = useReopenFeedback(stageId); const deleteMutation = useDeleteFeedback(stageId); const isPending = + updateMutation.isPending || resolveMutation.isPending || verifyMutation.isPending || reopenMutation.isPending || @@ -71,8 +63,10 @@ export function FeedbackChecklist({ const filteredItems = useMemo(() => { let result = [...items]; - if (severityFilter !== "ALL") { - result = result.filter((i: any) => i.severity === severityFilter); + if (typeFilter === "ACTION") { + result = result.filter((i: any) => i.isActionItem); + } else if (typeFilter === "INFO") { + result = result.filter((i: any) => !i.isActionItem); } if (statusFilter === "OPEN") { @@ -89,23 +83,25 @@ export function FeedbackChecklist({ } return result; - }, [items, severityFilter, statusFilter]); + }, [items, typeFilter, statusFilter]); - // Group by severity - const groupedItems = useMemo(() => { - const groups: Record = {}; - for (const sev of SEVERITY_ORDER) { - const group = filteredItems.filter((i: any) => i.severity === sev); - if (group.length > 0) groups[sev] = group; - } - return groups; - }, [filteredItems]); + // Separate action items and info callouts + const actionItems = useMemo( + () => filteredItems.filter((i: any) => i.isActionItem), + [filteredItems] + ); + const infoItems = useMemo( + () => filteredItems.filter((i: any) => !i.isActionItem), + [filteredItems] + ); - // Stats - const totalCount = items.length; - const resolvedCount = items.filter( + // Stats (only count action items for progress) + const allActionItems = items.filter((i: any) => i.isActionItem); + const totalCount = allActionItems.length; + const resolvedCount = allActionItems.filter( (i: any) => i.status === "RESOLVED" || i.status === "VERIFIED" ).length; + const infoCount = items.filter((i: any) => !i.isActionItem).length; const handleResolve = (itemId: string, resolutionNote?: string) => { resolveMutation.mutate( @@ -134,7 +130,14 @@ export function FeedbackChecklist({ }); }; - const hasActiveFilters = severityFilter !== "ALL" || statusFilter !== "ALL"; + const handleToggleType = (itemId: string, isActionItem: boolean) => { + updateMutation.mutate( + { itemId, data: { isActionItem } }, + { + onError: (err) => toast.error("Failed to update", { description: err.message }), + } + ); + }; return ( @@ -152,7 +155,7 @@ export function FeedbackChecklist({
- Feedback Checklist + Feedback {totalCount > 0 && ( )} + {infoCount > 0 && ( + + {infoCount} info + + )}
{collapsed ? ( @@ -177,7 +188,7 @@ export function FeedbackChecklist({ {!collapsed && (
- {/* Progress bar */} + {/* Progress bar (action items only) */} {totalCount > 0 && ( 0 && ( + {items.length > 0 && (
{/* Status filter */}
@@ -207,62 +218,35 @@ export function FeedbackChecklist({ ))}
- {/* Severity filter */} - - - - - - {(["ALL", ...SEVERITY_ORDER] as SeverityFilter[]).map( - (sev) => ( - - ) - )} - - - - {hasActiveFilters && ( - - )} + {t === "ALL" ? "All" : t === "ACTION" ? "Actions" : "Info"} + + ))} +
)} - {/* Item list grouped by severity */} + {/* Item list */} {isLoading ? (
Loading feedback items...
- ) : totalCount === 0 ? ( + ) : items.length === 0 ? (
No feedback items yet. Annotations will automatically create - checklist items. + action items.
) : filteredItems.length === 0 ? (
@@ -270,13 +254,14 @@ export function FeedbackChecklist({
) : (
- {Object.entries(groupedItems).map(([severity, group]) => ( -
+ {/* Action items first */} + {actionItems.length > 0 && ( +

- {severity} ({group.length}) + Action Items ({actionItems.length})

- {group.map((item: any) => ( + {actionItems.map((item: any) => ( ))}
- ))} + )} + + {/* Info callouts */} + {infoItems.length > 0 && ( +
+

+ Info Callouts ({infoItems.length}) +

+
+ {infoItems.map((item: any) => ( + + ))} +
+
+ )}
)}
diff --git a/src/components/review/feedback-item-card.tsx b/src/components/review/feedback-item-card.tsx index c891fae..4867a1b 100644 --- a/src/components/review/feedback-item-card.tsx +++ b/src/components/review/feedback-item-card.tsx @@ -6,10 +6,12 @@ import { CheckCheck, ChevronDown, ChevronUp, + Info, MapPin, RotateCcw, Trash2, ArrowRight, + CircleDot, } from "lucide-react"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; @@ -25,7 +27,7 @@ import { cn } from "@/lib/utils"; interface FeedbackItemData { id: string; summary: string; - severity: "CRITICAL" | "MAJOR" | "MINOR" | "SUGGESTION"; + isActionItem: boolean; status: "OPEN" | "IN_PROGRESS" | "RESOLVED" | "VERIFIED" | "REOPENED"; resolutionNote?: string | null; annotation?: { @@ -47,6 +49,7 @@ interface FeedbackItemCardProps { onVerify: (itemId: string) => void; onReopen: (itemId: string) => void; onDelete: (itemId: string) => void; + onToggleType?: (itemId: string, isActionItem: boolean) => void; onAnnotationClick?: (annotation: { id: string; imageX: number; @@ -55,25 +58,6 @@ interface FeedbackItemCardProps { isPending?: boolean; } -const SEVERITY_STYLES: Record = { - CRITICAL: { - badge: "bg-red-500/10 text-red-600 border-red-500/30", - border: "border-l-red-500", - }, - MAJOR: { - badge: "bg-orange-500/10 text-orange-600 border-orange-500/30", - border: "border-l-orange-500", - }, - MINOR: { - badge: "bg-yellow-500/10 text-yellow-600 border-yellow-500/30", - border: "border-l-yellow-500", - }, - SUGGESTION: { - badge: "bg-blue-500/10 text-blue-600 border-blue-500/30", - border: "border-l-blue-500", - }, -}; - const STATUS_LABELS: Record = { OPEN: "Open", IN_PROGRESS: "In Progress", @@ -88,6 +72,7 @@ export function FeedbackItemCard({ onVerify, onReopen, onDelete, + onToggleType, onAnnotationClick, isPending, }: FeedbackItemCardProps) { @@ -95,7 +80,6 @@ export function FeedbackItemCard({ const [resolutionNote, setResolutionNote] = useState(""); const isResolved = item.status === "RESOLVED" || item.status === "VERIFIED"; - const styles = SEVERITY_STYLES[item.severity]; const handleResolve = () => { onResolve(item.id, resolutionNote || undefined); @@ -107,37 +91,48 @@ export function FeedbackItemCard({
- {/* Checkbox */} - { - if (checked) { - if (item.summary.length < 50) { - onResolve(item.id); + {/* Checkbox (only for action items) */} + {item.isActionItem ? ( + { + if (checked) { + if (item.summary.length < 50) { + onResolve(item.id); + } else { + setExpanded(true); + } } else { - setExpanded(true); + onReopen(item.id); } - } else { - onReopen(item.id); - } - }} - className="mt-0.5" - /> + }} + className="mt-0.5" + /> + ) : ( + + )} {/* Content */}
- {item.severity} + {item.isActionItem ? "Action" : "Info"} {item.status !== "OPEN" && !isResolved && ( @@ -189,7 +184,7 @@ export function FeedbackItemCard({ )} {/* Expanded resolve form */} - {expanded && !isResolved && ( + {expanded && !isResolved && item.isActionItem && (