From 57c43e6f0cc2dee88fff6cbe532a8d6a7778f749 Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Mon, 13 Jul 2026 12:42:33 -0600 Subject: [PATCH 1/2] feat: polish review feedback threads --- apps/server/src/agents/reviews.ts | 71 ++- ...0030_review-feedback-resolution-states.sql | 11 + apps/server/src/db/seed/reviews.ts | 2 +- apps/server/src/routes/reviews.ts | 48 +- apps/server/src/server/mcp-review-handlers.ts | 2 +- .../shared/mcp/persona-interaction-tools.ts | 16 +- apps/server/src/shared/mcp/server.ts | 2 +- apps/server/test/review-feedback.test.ts | 45 +- apps/web/src/components/app/agents-view.tsx | 8 +- apps/web/src/components/app/changes-tab.tsx | 57 +- .../src/components/app/diff-annotations.tsx | 466 ++++++++++++---- apps/web/src/components/app/media-sidebar.tsx | 6 +- .../components/app/review-diff-snapshot.tsx | 59 ++ .../src/components/app/reviews-sidebar.tsx | 526 +++++++++++++----- .../src/components/app/unified-diff-view.tsx | 9 + apps/web/src/components/ui/button.tsx | 2 + apps/web/src/hooks/use-agent-reviews.ts | 53 ++ 17 files changed, 1091 insertions(+), 292 deletions(-) create mode 100644 apps/server/src/db/migrations/0030_review-feedback-resolution-states.sql create mode 100644 apps/web/src/components/app/review-diff-snapshot.tsx diff --git a/apps/server/src/agents/reviews.ts b/apps/server/src/agents/reviews.ts index b99152cd..ef278f3c 100644 --- a/apps/server/src/agents/reviews.ts +++ b/apps/server/src/agents/reviews.ts @@ -242,7 +242,7 @@ export async function resolveReviewFeedbackItem( pool: Pool, itemId: number, agentId: string, - resolution: "fixed" | "ignored" | "wont_fix", + resolution: "fixed" | "dismissed", opts: { note?: string | null; resolvedBy?: string | null } = {} ): Promise<{ item: ReviewFeedbackItemRecord; @@ -310,6 +310,70 @@ export async function resolveReviewFeedbackItem( } } +export async function reopenReviewFeedbackItem( + pool: Pool, + itemId: number, + agentId: string +): Promise<{ + item: ReviewFeedbackItemRecord; + reviewId: number; + reviewStatus: string; +} | null> { + const client = await pool.connect(); + try { + await client.query("BEGIN"); + const itemResult = await client.query( + `UPDATE review_feedback_items fi + SET status = 'open', resolution = NULL, resolution_note = NULL, + resolved_by = NULL, resolved_at = NULL, updated_at = NOW() + FROM reviews r + WHERE fi.id = $1 AND fi.review_id = r.id + AND (r.agent_id = $2 OR r.assigned_agent_id = $2) + RETURNING + fi.id, fi.review_id AS "reviewId", fi.file_path AS "filePath", + fi.line_start AS "lineStart", fi.line_end AS "lineEnd", + fi.diff_snapshot AS "diffSnapshot", fi.base_ref AS "baseRef", + fi.status, fi.resolution, fi.resolution_note AS "resolutionNote", + fi.resolved_by AS "resolvedBy", fi.resolved_at AS "resolvedAt", + fi.created_at AS "createdAt", fi.updated_at AS "updatedAt"`, + [itemId, agentId] + ); + const item = itemResult.rows[0]; + if (!item) { + await client.query("ROLLBACK"); + return null; + } + + const countsResult = await client.query<{ + total: number; + resolved: number; + }>( + `SELECT COUNT(*)::int AS total, + COUNT(*) FILTER (WHERE status = 'resolved')::int AS resolved + FROM review_feedback_items WHERE review_id = $1`, + [item.reviewId] + ); + const { total, resolved } = countsResult.rows[0]!; + const reviewStatus = + resolved === total + ? "resolved" + : resolved > 0 + ? "partially_resolved" + : "open"; + await client.query( + `UPDATE reviews SET status = $1, updated_at = NOW() WHERE id = $2`, + [reviewStatus, item.reviewId] + ); + await client.query("COMMIT"); + return { item, reviewId: item.reviewId, reviewStatus }; + } catch (err) { + await client.query("ROLLBACK"); + throw err; + } finally { + client.release(); + } +} + export async function addThreadMessage( pool: Pool, itemId: number, @@ -318,6 +382,11 @@ export async function addThreadMessage( body: string, authorAgentId?: string | null ): Promise<{ message: ReviewThreadMessageRecord; reviewId: number } | null> { + if (authorType === "agent" && body.length > 1_200) { + throw new Error( + "Agent review thread replies must be 1,200 characters or fewer." + ); + } const ownership = await pool.query<{ reviewId: number }>( `SELECT fi.review_id AS "reviewId" FROM review_feedback_items fi diff --git a/apps/server/src/db/migrations/0030_review-feedback-resolution-states.sql b/apps/server/src/db/migrations/0030_review-feedback-resolution-states.sql new file mode 100644 index 00000000..a228136a --- /dev/null +++ b/apps/server/src/db/migrations/0030_review-feedback-resolution-states.sql @@ -0,0 +1,11 @@ +-- The review-feedback system has two terminal states only. +ALTER TABLE review_feedback_items + DROP CONSTRAINT IF EXISTS review_feedback_items_resolution_check; + +UPDATE review_feedback_items +SET resolution = 'dismissed' +WHERE resolution IN ('ignored', 'wont_fix'); + +ALTER TABLE review_feedback_items + ADD CONSTRAINT review_feedback_items_resolution_check + CHECK (resolution IS NULL OR resolution IN ('fixed', 'dismissed')); diff --git a/apps/server/src/db/seed/reviews.ts b/apps/server/src/db/seed/reviews.ts index da469961..9c78be08 100644 --- a/apps/server/src/db/seed/reviews.ts +++ b/apps/server/src/db/seed/reviews.ts @@ -168,7 +168,7 @@ const REVIEWS: ReviewSeed[] = [ lineEnd: 60, diffSnapshot: null, status: "resolved", - resolution: "wont_fix", + resolution: "dismissed", resolutionNote: "Response shape matches the existing pattern used by other endpoints — changing it would be a breaking change.", messages: [ diff --git a/apps/server/src/routes/reviews.ts b/apps/server/src/routes/reviews.ts index 230463d9..551ecb0c 100644 --- a/apps/server/src/routes/reviews.ts +++ b/apps/server/src/routes/reviews.ts @@ -208,10 +208,10 @@ export async function registerReviewRoutes( "2. Read each feedback item. For each one, decide whether to fix it, push back, or dismiss it." ); notifLines.push( - "3. If you have a question or want to explain your approach before acting, use dispatch_review_add_message to reply on that item's thread." + "3. If you have a question or need to explain your approach, use dispatch_review_add_message to reply on that item's thread. Keep replies to 1–3 short sentences (max 1,200 characters)." ); notifLines.push( - "4. After addressing an item (or deciding not to), call dispatch_review_resolve to mark it as fixed, ignored, or wont_fix. Include a note when dismissing so the reviewer understands why." + "4. After addressing an item (or deciding not to), call dispatch_review_resolve to mark it as fixed or dismissed. Include a brief note when dismissing so the reviewer understands why." ); notifLines.push( "5. Work through all items — the review status updates automatically as you resolve each one." @@ -246,13 +246,14 @@ export async function registerReviewRoutes( note?: unknown; } | null; - const validResolutions = ["fixed", "ignored", "wont_fix"] as const; + const validResolutions = ["fixed", "dismissed"] as const; if ( - typeof body?.resolution !== "string" || - !(validResolutions as readonly string[]).includes(body.resolution) + body?.resolution !== null && + (typeof body?.resolution !== "string" || + !(validResolutions as readonly string[]).includes(body.resolution)) ) { return reply.code(400).send({ - error: "resolution must be one of: fixed, ignored, wont_fix", + error: "resolution must be fixed, dismissed, or null", }); } @@ -270,13 +271,20 @@ export async function registerReviewRoutes( const agent = await deps.agentManager.getAgent(agentId); if (!agent) return reply.code(404).send({ error: "Agent not found." }); - const result = await reviewQueries.resolveReviewFeedbackItem( - deps.pool, - itemId, - agentId, - body.resolution as "fixed" | "ignored" | "wont_fix", - { note } - ); + const result = + body.resolution === null + ? await reviewQueries.reopenReviewFeedbackItem( + deps.pool, + itemId, + agentId + ) + : await reviewQueries.resolveReviewFeedbackItem( + deps.pool, + itemId, + agentId, + body.resolution as "fixed" | "dismissed", + { note } + ); if (!result) { return reply.code(404).send({ error: "Feedback item not found." }); } @@ -347,6 +355,20 @@ export async function registerReviewRoutes( feedbackItemId: itemId, }); + try { + await deps.sendAgentPrompt( + agentId, + [ + "--- DISPATCH: Review Thread Reply ---", + `Feedback item #${itemId}: ${body.body.trim()}`, + "Reply only if useful. Keep any reply to 1–3 short sentences (max 1,200 characters).", + "--- END ---", + ].join("\n") + ); + } catch { + // tmux delivery is best-effort + } + return { message: result.message }; } catch (error) { return deps.handleAgentError(reply, error); diff --git a/apps/server/src/server/mcp-review-handlers.ts b/apps/server/src/server/mcp-review-handlers.ts index cb72aa54..24c5dc21 100644 --- a/apps/server/src/server/mcp-review-handlers.ts +++ b/apps/server/src/server/mcp-review-handlers.ts @@ -320,7 +320,7 @@ export function createReviewHandlers(deps: CreateReviewHandlersDeps) { async resolveReviewFeedback( agentId: string, itemId: number, - resolution: "fixed" | "ignored" | "wont_fix", + resolution: "fixed" | "dismissed", opts: { note?: string | null } = {} ): Promise<{ item: { diff --git a/apps/server/src/shared/mcp/persona-interaction-tools.ts b/apps/server/src/shared/mcp/persona-interaction-tools.ts index 8ab24fa1..a1b269ba 100644 --- a/apps/server/src/shared/mcp/persona-interaction-tools.ts +++ b/apps/server/src/shared/mcp/persona-interaction-tools.ts @@ -292,7 +292,7 @@ export function registerPersonaInteractionTools( "dispatch_review_resolve", { description: - "Resolve a human review feedback item. Marks the item as fixed, dismissed, or wont_fix. The parent review status is automatically recomputed (open → partially_resolved → resolved).", + "Resolve a human review feedback item. Marks the item as fixed or dismissed. The parent review status is automatically recomputed (open → partially_resolved → resolved).", inputSchema: { itemId: z .number() @@ -300,16 +300,16 @@ export function registerPersonaInteractionTools( .positive() .describe("The ID of the review feedback item to resolve."), resolution: z - .enum(["fixed", "ignored", "wont_fix"]) + .enum(["fixed", "dismissed"]) .describe( - "Resolution type: 'fixed' if addressed, 'ignored' if not applicable, 'wont_fix' if acknowledged but intentionally left." + "Resolution type: 'fixed' if addressed, 'dismissed' if closing without a change." ), note: z .string() .max(10_000) .optional() .describe( - "Optional note explaining the resolution. Encouraged for 'dismissed' and 'wont_fix'." + "Optional note explaining the resolution. Encouraged when dismissed." ), }, }, @@ -347,7 +347,7 @@ export function registerPersonaInteractionTools( "dispatch_review_add_message", { description: - "Add a message to a review feedback item's thread. Use this to reply to a human review comment — for example, to ask a clarifying question or explain your approach before resolving.", + "Add a concise message to a review feedback item's thread. Use this to ask a clarifying question or explain your approach before resolving. Keep it to 1–3 short sentences (maximum 1,200 characters).", inputSchema: { itemId: z .number() @@ -359,8 +359,10 @@ export function registerPersonaInteractionTools( body: z .string() .min(1) - .max(50_000) - .describe("The message body (plain text or markdown)."), + .max(1_200) + .describe( + "A brief plain-text or Markdown reply (1–3 short sentences)." + ), }, }, async (args) => { diff --git a/apps/server/src/shared/mcp/server.ts b/apps/server/src/shared/mcp/server.ts index 319c6c5d..34bbea76 100644 --- a/apps/server/src/shared/mcp/server.ts +++ b/apps/server/src/shared/mcp/server.ts @@ -348,7 +348,7 @@ export type McpRequestContext = { resolveReviewFeedback?: ( agentId: string, itemId: number, - resolution: "fixed" | "ignored" | "wont_fix", + resolution: "fixed" | "dismissed", opts?: { note?: string | null } ) => Promise<{ item: { id: number; reviewId: number; status: string; resolution: string }; diff --git a/apps/server/test/review-feedback.test.ts b/apps/server/test/review-feedback.test.ts index b69aa7b4..9312635c 100644 --- a/apps/server/test/review-feedback.test.ts +++ b/apps/server/test/review-feedback.test.ts @@ -96,7 +96,7 @@ describe("PATCH /api/v1/agents/:id/reviews/items/:itemId", () => { expect(result.item.resolution).toBe("fixed"); }); - it("resolves a feedback item as ignored with a note", async () => { + it("resolves a feedback item as dismissed with a note", async () => { const review = await createReview(AGENT_ID, [{ comment: "Do this" }]); const itemId = review.items[0].id; @@ -104,27 +104,24 @@ describe("PATCH /api/v1/agents/:id/reviews/items/:itemId", () => { method: "PATCH", url: `/api/v1/agents/${AGENT_ID}/reviews/items/${itemId}`, headers: { cookie: sessionCookie, "content-type": "application/json" }, - payload: { resolution: "ignored", note: "Not applicable" }, + payload: { resolution: "dismissed", note: "Not applicable" }, }); expect(response.statusCode).toBe(200); - expect(response.json().item.resolution).toBe("ignored"); + expect(response.json().item.resolution).toBe("dismissed"); expect(response.json().item.resolutionNote).toBe("Not applicable"); }); - it("resolves a feedback item as wont_fix", async () => { + it("rejects unsupported review resolutions", async () => { const review = await createReview(AGENT_ID, [{ comment: "Refactor" }]); - const itemId = review.items[0].id; - const response = await ctx.app.inject({ method: "PATCH", - url: `/api/v1/agents/${AGENT_ID}/reviews/items/${itemId}`, + url: `/api/v1/agents/${AGENT_ID}/reviews/items/${review.items[0].id}`, headers: { cookie: sessionCookie, "content-type": "application/json" }, - payload: { resolution: "wont_fix", note: "By design" }, + payload: { resolution: "wont_fix" }, }); - expect(response.statusCode).toBe(200); - expect(response.json().item.resolution).toBe("wont_fix"); + expect(response.statusCode).toBe(400); }); it("updates review status to resolved when all items are resolved", async () => { @@ -171,6 +168,34 @@ describe("PATCH /api/v1/agents/:id/reviews/items/:itemId", () => { expect(reviewResponse.json().review.status).toBe("partially_resolved"); }); + it("reopens a resolved feedback item", async () => { + const review = await createReview(AGENT_ID, [{ comment: "Bug 1" }]); + const itemId = review.items[0].id; + + await ctx.app.inject({ + method: "PATCH", + url: `/api/v1/agents/${AGENT_ID}/reviews/items/${itemId}`, + headers: { cookie: sessionCookie, "content-type": "application/json" }, + payload: { resolution: "fixed" }, + }); + const response = await ctx.app.inject({ + method: "PATCH", + url: `/api/v1/agents/${AGENT_ID}/reviews/items/${itemId}`, + headers: { cookie: sessionCookie, "content-type": "application/json" }, + payload: { resolution: null }, + }); + + expect(response.statusCode).toBe(200); + expect(response.json().item.status).toBe("open"); + expect(response.json().item.resolution).toBeNull(); + const reviewResponse = await ctx.app.inject({ + method: "GET", + url: `/api/v1/agents/${AGENT_ID}/reviews/${review.id}`, + headers: { cookie: sessionCookie }, + }); + expect(reviewResponse.json().review.status).toBe("open"); + }); + it("returns 404 for item belonging to different agent", async () => { const review = await createReview(AGENT_ID, [{ comment: "Private" }]); const itemId = review.items[0].id; diff --git a/apps/web/src/components/app/agents-view.tsx b/apps/web/src/components/app/agents-view.tsx index b5f9ba8d..9a410365 100644 --- a/apps/web/src/components/app/agents-view.tsx +++ b/apps/web/src/components/app/agents-view.tsx @@ -368,16 +368,20 @@ export function AgentsView({ }, []); const handleNavigateToFile = useCallback( - (filePath: string, lineStart: number | null) => { + (filePath: string, lineStart: number | null, feedbackItemId?: number) => { if (!focusedAgentId) return; const params = new URLSearchParams(); params.set("file", filePath); if (lineStart != null) params.set("line", String(lineStart)); + if (feedbackItemId != null) { + params.set("feedback", String(feedbackItemId)); + } navTo(`/agents/${focusedAgentId}/changes?${params.toString()}`, { replace: true, }); + if (isMobile) setMobileMediaOpen(false); }, - [focusedAgentId, navTo] + [focusedAgentId, isMobile, navTo, setMobileMediaOpen] ); const handleReviewSubmitted = useCallback( diff --git a/apps/web/src/components/app/changes-tab.tsx b/apps/web/src/components/app/changes-tab.tsx index cd78489f..24509ae6 100644 --- a/apps/web/src/components/app/changes-tab.tsx +++ b/apps/web/src/components/app/changes-tab.tsx @@ -164,6 +164,14 @@ export const ChangesTab = memo(function ChangesTab({ ); const [selectedFile, setSelectedFile] = useState(null); + const [focusedFeedbackItemId, setFocusedFeedbackItemId] = useState< + number | null + >(null); + const handleFeedbackFocusComplete = useCallback((feedbackItemId: number) => { + setFocusedFeedbackItemId((current) => + current === feedbackItemId ? null : current + ); + }, []); const [lineSelection, setLineSelection] = useState( null ); @@ -201,15 +209,23 @@ export const ChangesTab = memo(function ChangesTab({ const [searchParams, setSearchParams] = useSearchParams(); const navFileTarget = searchParams.get("file"); const navLineTarget = searchParams.get("line"); + const navFeedbackTarget = searchParams.get("feedback"); useEffect(() => { if (!navFileTarget || files.length === 0) return; const targetFile = files.find((f) => f.path === navFileTarget); + if (isMobile) setFileTreeOpen(false); + const feedbackItemId = navFeedbackTarget ? Number(navFeedbackTarget) : null; + setFocusedFeedbackItemId( + feedbackItemId != null && Number.isInteger(feedbackItemId) + ? feedbackItemId + : null + ); setSearchParams({}, { replace: true }); if (targetFile) { requestAnimationFrame(() => { scrollToFile(navFileTarget); - if (navLineTarget && targetFile.diff) { + if (!navFeedbackTarget && navLineTarget && targetFile.diff) { const lineNum = Number(navLineTarget); if (Number.isInteger(lineNum) && lineNum > 0) { requestAnimationFrame(() => { @@ -224,10 +240,12 @@ export const ChangesTab = memo(function ChangesTab({ lineNum ); if (changeKey) { - const el = scrollRef.current?.querySelector( - `[id="${CSS.escape(changeKey)}"]` + const changeCell = scrollRef.current?.querySelector( + `[data-change-key="${CSS.escape(changeKey)}"]` ); - el?.scrollIntoView({ block: "center", behavior: "smooth" }); + changeCell + ?.closest("tr") + ?.scrollIntoView({ block: "center", behavior: "smooth" }); } } catch { // diff parse failed — fall back to file-level scroll @@ -237,7 +255,16 @@ export const ChangesTab = memo(function ChangesTab({ } }); } - }, [navFileTarget, navLineTarget, files, scrollToFile, setSearchParams]); + }, [ + navFileTarget, + navLineTarget, + navFeedbackTarget, + files, + isMobile, + scrollToFile, + setFileTreeOpen, + setSearchParams, + ]); const scrollRef = useRef(null); const scrollTimerRef = useRef | null>(null); @@ -341,6 +368,8 @@ export const ChangesTab = memo(function ChangesTab({ onUpdateDraft={updateDraft} onStartReview={() => setReviewMode(true)} feedbackItems={feedbackItems} + focusedFeedbackItemId={focusedFeedbackItemId} + onFeedbackFocusComplete={handleFeedbackFocusComplete} /> @@ -602,6 +631,8 @@ type DiffPaneProps = { onUpdateDraft?: (id: string, comment: string) => void; onStartReview?: () => void; feedbackItems?: ReviewFeedbackItem[]; + focusedFeedbackItemId?: number | null; + onFeedbackFocusComplete?: (feedbackItemId: number) => void; }; function DiffPane({ @@ -625,6 +656,8 @@ function DiffPane({ onUpdateDraft, onStartReview, feedbackItems, + focusedFeedbackItemId, + onFeedbackFocusComplete, }: DiffPaneProps): JSX.Element { return (
fi.filePath === file.path )} + focusedFeedbackItemId={focusedFeedbackItemId} + onFeedbackFocusComplete={onFeedbackFocusComplete} /> ))}
@@ -691,6 +726,8 @@ type FileDiffSectionProps = { onUpdateDraft?: (id: string, comment: string) => void; onStartReview?: () => void; feedbackItems?: ReviewFeedbackItem[]; + focusedFeedbackItemId?: number | null; + onFeedbackFocusComplete?: (feedbackItemId: number) => void; }; function FileDiffSection({ @@ -712,6 +749,8 @@ function FileDiffSection({ onUpdateDraft, onStartReview, feedbackItems, + focusedFeedbackItemId, + onFeedbackFocusComplete, }: FileDiffSectionProps): JSX.Element { return (
@@ -770,6 +809,8 @@ function FileDiffSection({ onUpdateDraft={onUpdateDraft} onStartReview={onStartReview} feedbackItems={feedbackItems} + focusedFeedbackItemId={focusedFeedbackItemId} + onFeedbackFocusComplete={onFeedbackFocusComplete} /> )} @@ -799,6 +840,8 @@ type FileDiffContentProps = { onUpdateDraft?: (id: string, comment: string) => void; onStartReview?: () => void; feedbackItems?: ReviewFeedbackItem[]; + focusedFeedbackItemId?: number | null; + onFeedbackFocusComplete?: (feedbackItemId: number) => void; }; function FileDiffContent({ @@ -817,6 +860,8 @@ function FileDiffContent({ onUpdateDraft, onStartReview, feedbackItems, + focusedFeedbackItemId, + onFeedbackFocusComplete, }: FileDiffContentProps): JSX.Element { const [forceLoad, setForceLoad] = useState(false); const { data: fileDiffData, isLoading: fileDiffLoading } = useAgentFileDiff( @@ -888,6 +933,8 @@ function FileDiffContent({ onUpdateDraft={onUpdateDraft} onStartReview={onStartReview} feedbackItems={feedbackItems} + focusedFeedbackItemId={focusedFeedbackItemId} + onFeedbackFocusComplete={onFeedbackFocusComplete} /> ); } diff --git a/apps/web/src/components/app/diff-annotations.tsx b/apps/web/src/components/app/diff-annotations.tsx index b0cb65b5..a2db173a 100644 --- a/apps/web/src/components/app/diff-annotations.tsx +++ b/apps/web/src/components/app/diff-annotations.tsx @@ -1,24 +1,44 @@ -import { useCallback, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { useNavigate } from "react-router-dom"; import { + Check, CheckCircle2, ChevronDown, ChevronRight, + Circle, Loader2, MessageCircle, MessageSquare, MessageSquarePlus, MoreHorizontal, Pencil, + RotateCcw, + Send, Trash2, + XCircle, } from "lucide-react"; import { AnimatePresence, motion } from "framer-motion"; +import { toast } from "sonner"; import { type ReviewFeedbackItem } from "@/hooks/use-agent-reviews"; +import { + useAddReviewThreadMessage, + useSetReviewFeedbackResolution, + type ReviewThreadMessage, +} from "@/hooks/use-agent-reviews"; import type { PersistedDraftComment } from "@/lib/store"; import { agentRoute } from "@/lib/agent-routes"; import { api } from "@/lib/api"; import { cn } from "@/lib/utils"; +import { Button } from "@/components/ui/button"; +import { Textarea } from "@/components/ui/textarea"; +import { Markdown } from "@/components/ui/markdown"; +import { + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from "@/components/ui/tooltip"; import { DropdownMenu, DropdownMenuContent, @@ -198,17 +218,42 @@ export function InlineCommentForm({ } export function InlineFeedbackAnnotation({ + agentId, comment, isResolved, feedbackItem, + focused = false, + onFocusComplete, }: { + agentId: string | null; feedbackItem: ReviewFeedbackItem; comment: string; isResolved: boolean; + focused?: boolean; + onFocusComplete?: (feedbackItemId: number) => void; }): JSX.Element { - const [expanded, setExpanded] = useState(!isResolved); + const [expanded, setExpanded] = useState(false); + const annotationRef = useRef(null); + const [reply, setReply] = useState(""); + const [replying, setReplying] = useState(false); + const addMessage = useAddReviewThreadMessage(agentId); + const setResolution = useSetReviewFeedbackResolution(agentId); - const statusLabel = isResolved ? "Resolved" : "Pending"; + const state = !isResolved + ? "open" + : feedbackItem.resolution === "fixed" + ? "fixed" + : "dismissed"; + const statusLabel = + state === "fixed" ? "Fixed" : state === "dismissed" ? "Dismissed" : "Open"; + const statusIcon = + state === "fixed" ? ( + + ) : state === "dismissed" ? ( + + ) : ( + + ); const lineLabel = feedbackItem.lineStart != null ? feedbackItem.lineEnd && feedbackItem.lineEnd !== feedbackItem.lineStart @@ -216,109 +261,237 @@ export function InlineFeedbackAnnotation({ : `Line ${feedbackItem.lineStart}` : null; + useEffect(() => { + if (!focused) return; + setExpanded(true); + const frame = requestAnimationFrame(() => { + annotationRef.current?.scrollIntoView({ + behavior: "smooth", + block: "center", + }); + }); + const settleTimer = window.setTimeout(() => { + annotationRef.current?.scrollIntoView({ + behavior: "smooth", + block: "center", + }); + onFocusComplete?.(feedbackItem.id); + }, 220); + return () => { + cancelAnimationFrame(frame); + window.clearTimeout(settleTimer); + }; + }, [feedbackItem.id, focused, onFocusComplete]); + + const submitReply = async (event: React.FormEvent) => { + event.preventDefault(); + const body = reply.trim(); + if (!body) return; + try { + await addMessage.mutateAsync({ itemId: feedbackItem.id, body }); + setReply(""); + setReplying(false); + } catch { + toast.error("Couldn't send the reply. Try again."); + } + }; + + const cancelReply = () => { + setReply(""); + setReplying(false); + }; + + const updateResolution = async (resolution: "fixed" | "dismissed" | null) => { + try { + await setResolution.mutateAsync({ + itemId: feedbackItem.id, + resolution, + }); + } catch { + toast.error("Couldn't update the feedback state. Try again."); + } + }; + return ( -
+
setExpanded((v) => !v)} - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.preventDefault(); - setExpanded((v) => !v); - } - }} + ref={annotationRef} + data-review-feedback-id={feedbackItem.id} + className="sticky left-0 my-3 ml-3 max-w-full overflow-clip rounded-md bg-card shadow-sm ring-1 ring-border/50" + style={stickyAnnotationStyle} > - - Feedback - - {statusLabel} - - {lineLabel && ( - {lineLabel} - )} - -
-
-

- {comment} -

-
- - {expanded && - (feedbackItem.messages.length > 1 || - feedbackItem.diffSnapshot || - feedbackItem.resolution) && ( +
+ + + {comment} + +
+ + {expanded && ( -
- {feedbackItem.diffSnapshot && ( - 
-                    {feedbackItem.diffSnapshot}
-
- )} - {feedbackItem.messages.slice(1).map((msg) => ( -
-
- - {msg.authorType === "human" ? "Human" : "Agent"} - - · - - {new Date(msg.createdAt).toLocaleTimeString(undefined, { - hour: "2-digit", - minute: "2-digit", - })} - -
-

- {msg.content?.body} -

-
- ))} +
+ {feedbackItem.messages + .slice(1) + .map((message, index, messages) => ( + 0 && + messages[index - 1]?.authorType === message.authorType + } + /> + ))} + + {replying ? ( + +