Skip to content

Commit d0ec7b5

Browse files
committed
Refine PR review UX and surface agent findings
- Add PR review commands to the command palette - Tighten file comment validation and line entry UX - Show agent review and rule violation banners with inline findings
1 parent 4fad841 commit d0ec7b5

4 files changed

Lines changed: 160 additions & 16 deletions

File tree

apps/web/src/components/CommandPalette.tsx

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { ThreadId } from "@okcode/contracts";
2-
import { useNavigate, useParams } from "@tanstack/react-router";
2+
import { useNavigate, useParams, useRouterState } from "@tanstack/react-router";
33
import { useCallback, useMemo, useRef, useState } from "react";
44
import {
55
ArrowLeftIcon,
@@ -22,6 +22,9 @@ import type { LucideIcon } from "lucide-react";
2222
import { cn } from "~/lib/utils";
2323
import { useStore } from "~/store";
2424
import { useCommandPaletteStore } from "~/commandPaletteStore";
25+
import { usePrReviewCommands } from "~/components/pr-review/usePrReviewCommands";
26+
import { usePrReviewStore } from "~/prReviewStore";
27+
import { ensureNativeApi } from "~/nativeApi";
2528
import { useHandleNewThread } from "~/hooks/useHandleNewThread";
2629
import { useCurrentWorktreeCleanupCandidates } from "~/hooks/useCurrentWorktreeCleanupCandidates";
2730
import { useTheme } from "~/hooks/useTheme";
@@ -147,6 +150,25 @@ function CommandsView() {
147150
const mruThreadIds = useCommandPaletteStore((s) => s.mruThreadIds);
148151
const openWorktreeCleanupDialog = useWorktreeCleanupStore((s) => s.openDialog);
149152
const { hasCandidates: hasWorktreeCleanupCandidates } = useCurrentWorktreeCleanupCandidates();
153+
154+
// PR Review commands integration
155+
const currentPath = useRouterState({ select: (s) => s.location.pathname });
156+
const isPrReviewRoute = currentPath.includes("/pr-review");
157+
const prReviewDashboard = usePrReviewStore((s) => s.selectedPrNumber);
158+
const prReviewCommands = usePrReviewCommands({
159+
enabled: isPrReviewRoute,
160+
onStartAgentReview: () => {
161+
closePalette();
162+
const store = usePrReviewStore.getState();
163+
// Trigger is handled by the caller
164+
document.dispatchEvent(new CustomEvent("command-palette:start-agent-review"));
165+
},
166+
onOpenOnGitHub: () => {
167+
closePalette();
168+
document.dispatchEvent(new CustomEvent("command-palette:open-on-github"));
169+
},
170+
});
171+
150172
const routeThreadId = useParams({
151173
strict: false,
152174
select: (params) => (params.threadId ? ThreadId.makeUnsafe(params.threadId) : null),
@@ -371,6 +393,11 @@ function CommandsView() {
371393
},
372394
});
373395

396+
// ── PR Review (conditionally visible) ──
397+
for (const prCmd of prReviewCommands) {
398+
cmds.push(prCmd);
399+
}
400+
374401
return cmds.filter((cmd) => !cmd.hidden);
375402
}, [
376403
projects,
@@ -389,6 +416,7 @@ function CommandsView() {
389416
pushMruThread,
390417
openWorktreeCleanupDialog,
391418
hasWorktreeCleanupCandidates,
419+
prReviewCommands,
392420
]);
393421

394422
// Filter commands by query

apps/web/src/components/pr-review/PrFileCommentComposer.tsx

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { useEffect, useState } from "react";
33
import { MessageSquareIcon } from "lucide-react";
44
import { useLocalStorage } from "~/hooks/useLocalStorage";
55
import { Button } from "~/components/ui/button";
6-
import { Input } from "~/components/ui/input";
76
import { Spinner } from "~/components/ui/spinner";
87
import { PrMentionComposer } from "./PrMentionComposer";
98
import { TEXT_DRAFT_SCHEMA } from "./pr-review-utils";
@@ -32,22 +31,25 @@ export function PrFileCommentComposer({
3231
setLine(String(defaultLine));
3332
}, [defaultLine]);
3433

34+
const parsedLine = Number.parseInt(line, 10);
35+
const isValidLine = Number.isFinite(parsedLine) && parsedLine >= 1;
36+
3537
return (
3638
<div className="space-y-3 rounded-2xl border border-border/70 bg-background/90 p-3">
3739
<div className="flex items-center gap-3">
38-
<label className="flex items-center gap-2 text-xs font-medium text-muted-foreground">
39-
Line
40-
<Input
41-
className="h-8 w-24"
40+
<span className="text-xs text-muted-foreground">
41+
Comment on line{" "}
42+
<input
43+
className="inline-flex h-6 w-14 rounded border border-border/70 bg-muted/30 px-1.5 text-center text-xs font-medium text-foreground tabular-nums focus:border-primary/50 focus:outline-none focus:ring-1 focus:ring-ring/24"
4244
disabled={disabled || isSubmitting}
4345
inputMode="numeric"
4446
min={1}
4547
type="number"
4648
value={line}
4749
onChange={(event) => setLine(event.target.value)}
48-
/>
49-
</label>
50-
<span className="text-xs text-muted-foreground">Creates a review thread on {path}</span>
50+
/>{" "}
51+
of <span className="font-medium text-foreground">{path}</span>
52+
</span>
5153
</div>
5254
<PrMentionComposer
5355
cwd={cwd}
@@ -59,12 +61,11 @@ export function PrFileCommentComposer({
5961
/>
6062
<div className="flex items-center justify-end gap-2">
6163
<Button
62-
disabled={disabled || isSubmitting || body.trim().length === 0}
64+
disabled={disabled || isSubmitting || body.trim().length === 0 || !isValidLine}
6365
onClick={() => {
64-
const nextLine = Number.parseInt(line, 10);
65-
if (!Number.isFinite(nextLine) || nextLine < 1) return;
66+
if (!isValidLine) return;
6667
setIsSubmitting(true);
67-
void onSubmit({ body: body.trim(), line: nextLine }).then(
68+
void onSubmit({ body: body.trim(), line: parsedLine }).then(
6869
() => {
6970
setBody("");
7071
setIsSubmitting(false);

apps/web/src/components/pr-review/PrReviewShell.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { PanelRightIcon } from "lucide-react";
66
import { useAppSettings } from "~/appSettings";
77
import { useMediaQuery } from "~/hooks/useMediaQuery";
88
import { invalidatePrReviewQueries } from "~/lib/prReviewReactQuery";
9-
import { cn } from "~/lib/utils";
109
import { ensureNativeApi } from "~/nativeApi";
1110
import { joinPath } from "~/components/review/reviewUtils";
1211
import { Button } from "~/components/ui/button";
@@ -394,7 +393,6 @@ export function PrReviewShell({
394393
requestChangesVariant={resolveRequestChangesButtonVariant(
395394
settings.prReviewRequestChangesTone,
396395
)}
397-
reviewComposerRef={reviewComposerRef}
398396
/>
399397

400398
{/* Inspector sheet (mobile/tablet) */}

apps/web/src/components/pr-review/PrWorkspace.tsx

Lines changed: 118 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { FileDiff, Virtualizer } from "@pierre/diffs/react";
2-
import type { NativeApi, PrReviewThread } from "@okcode/contracts";
2+
import type { NativeApi, PrAgentReviewResult, PrReviewThread } from "@okcode/contracts";
33
import { useMemo } from "react";
44
import { useQuery } from "@tanstack/react-query";
55
import { Schema } from "effect";
@@ -23,6 +23,8 @@ import { useFileViewNavigation } from "~/hooks/useFileViewNavigation";
2323
import { joinPath } from "~/components/review/reviewUtils";
2424
import { projectPathExistsQueryOptions } from "~/lib/projectReactQuery";
2525
import type { Project } from "~/types";
26+
import { PrAgentReviewBanner } from "./PrAgentReviewBanner";
27+
import { PrRuleViolationBanner } from "./PrRuleViolationBanner";
2628
import { PrFileCommentComposer } from "./PrFileCommentComposer";
2729
import { PrFileTabStrip } from "./PrFileTabStrip";
2830
import {
@@ -43,24 +45,34 @@ export function PrWorkspace({
4345
project,
4446
patch,
4547
dashboard,
48+
agentResult,
49+
onStartAgentReview,
50+
isStartingAgentReview,
4651
selectedFilePath,
4752
selectedThreadId,
4853
reviewedFiles,
54+
approvalBlockers,
4955
onSelectFilePath,
5056
onSelectThreadId,
5157
onCreateThread,
5258
onToggleFileReviewed,
59+
onOpenConflictDrawer,
5360
}: {
5461
project: Project;
5562
patch: string | null;
5663
dashboard: Awaited<ReturnType<NativeApi["prReview"]["getDashboard"]>> | null | undefined;
64+
agentResult: PrAgentReviewResult | null | undefined;
65+
onStartAgentReview: () => void;
66+
isStartingAgentReview: boolean;
5767
selectedFilePath: string | null;
5868
selectedThreadId: string | null;
5969
reviewedFiles: readonly string[];
70+
approvalBlockers: string[];
6071
onSelectFilePath: (path: string) => void;
6172
onSelectThreadId: (threadId: string | null) => void;
6273
onCreateThread: (input: { path: string; line: number; body: string }) => Promise<void>;
6374
onToggleFileReviewed: (path: string) => void;
75+
onOpenConflictDrawer: () => void;
6476
}) {
6577
const { resolvedTheme } = useTheme();
6678
const openFileInCodeViewer = useFileViewNavigation();
@@ -91,6 +103,17 @@ export function PrWorkspace({
91103
}, {});
92104
}, [dashboard]);
93105

106+
// Agent findings grouped by file path
107+
const agentFindingsByPath = useMemo<Record<string, typeof agentResult extends { findings: infer F } ? F : never>>(() => {
108+
if (!agentResult?.findings) return {};
109+
return agentResult.findings.reduce<Record<string, typeof agentResult.findings>>((acc, finding) => {
110+
if (!finding.path) return acc;
111+
if (!acc[finding.path]) acc[finding.path] = [];
112+
acc[finding.path]!.push(finding);
113+
return acc;
114+
}, {});
115+
}, [agentResult?.findings]);
116+
94117
const patchFiles = useMemo(
95118
() => (renderablePatch?.kind === "files" ? renderablePatch.files : []),
96119
[renderablePatch],
@@ -184,6 +207,24 @@ export function PrWorkspace({
184207
</Button>
185208
</div>
186209

210+
{/* Agent review banner */}
211+
<PrAgentReviewBanner
212+
agentStatus={agentResult}
213+
onStartReview={onStartAgentReview}
214+
onSelectFile={onSelectFilePath}
215+
onOpenFindings={() => {
216+
// Handled by inspector tab switch via store
217+
}}
218+
isStarting={isStartingAgentReview}
219+
fileCount={dashboard.files.length}
220+
/>
221+
222+
{/* Rule violation banner */}
223+
<PrRuleViolationBanner
224+
approvalBlockers={approvalBlockers}
225+
onOpenConflictDrawer={onOpenConflictDrawer}
226+
/>
227+
187228
{/* File tab strip */}
188229
{patchFiles.length > 0 ? (
189230
<PrFileTabStrip
@@ -220,6 +261,7 @@ export function PrWorkspace({
220261
const filePath = resolveFileDiffPath(fileDiff);
221262
const fileKey = `${buildFileDiffRenderKey(fileDiff)}:${resolvedTheme}`;
222263
const fileThreads = threadsByPath[filePath] ?? [];
264+
const fileFindings = agentFindingsByPath[filePath] ?? [];
223265
const isSelected = selectedFilePath === filePath;
224266
const isReviewed = reviewedFilesSet.has(filePath);
225267
const firstCommentLine = fileThreads[0]?.line ?? 1;
@@ -271,6 +313,13 @@ export function PrWorkspace({
271313
+{fileThreads.length - 2} more
272314
</span>
273315
) : null}
316+
{/* Agent findings indicator for this file */}
317+
{fileFindings.length > 0 ? (
318+
<span className="inline-flex items-center gap-1 rounded-full border border-indigo-500/20 bg-indigo-500/8 px-2.5 py-0.5 text-[11px] font-medium text-indigo-400">
319+
<SparklesIconInline />
320+
{fileFindings.length} finding{fileFindings.length === 1 ? "" : "s"}
321+
</span>
322+
) : null}
274323
<button
275324
className={cn(
276325
"inline-flex items-center gap-1 rounded-full border px-2.5 py-0.5 text-[11px] font-medium transition-colors",
@@ -303,6 +352,53 @@ export function PrWorkspace({
303352
</Button>
304353
</div>
305354
</div>
355+
356+
{/* Inline agent findings for this file */}
357+
{fileFindings.length > 0 ? (
358+
<div className="border-b border-indigo-500/15 bg-indigo-500/5 px-4 py-2 space-y-1.5">
359+
{fileFindings.map((finding) => (
360+
<div
361+
className="flex items-start gap-2 text-xs"
362+
key={finding.id}
363+
>
364+
<span
365+
className={cn(
366+
"mt-0.5 shrink-0 size-3.5 rounded-full flex items-center justify-center text-[8px] font-bold",
367+
finding.severity === "critical"
368+
? "bg-rose-500/20 text-rose-400"
369+
: finding.severity === "warning"
370+
? "bg-amber-500/20 text-amber-400"
371+
: "bg-sky-500/20 text-sky-400",
372+
)}
373+
>
374+
{finding.severity === "critical" ? "!" : finding.severity === "warning" ? "!" : "i"}
375+
</span>
376+
<div className="min-w-0 flex-1">
377+
<span className="font-medium text-foreground">{finding.title}</span>
378+
{finding.line ? (
379+
<span className="ml-1.5 text-muted-foreground">L{finding.line}</span>
380+
) : null}
381+
</div>
382+
<button
383+
className="shrink-0 text-[11px] text-indigo-400 hover:text-indigo-300"
384+
onClick={() => {
385+
if (finding.path && finding.line) {
386+
void onCreateThread({
387+
path: finding.path,
388+
line: finding.line,
389+
body: `**AI Finding (${finding.severity}):** ${finding.title}\n\n${finding.detail}`,
390+
});
391+
}
392+
}}
393+
type="button"
394+
>
395+
Create thread
396+
</button>
397+
</div>
398+
))}
399+
</div>
400+
) : null}
401+
306402
<div className="p-3">
307403
<FileDiff
308404
fileDiff={fileDiff}
@@ -385,3 +481,24 @@ function PrDiffFileHeaderBadge({ cwd, path }: { cwd: string; path: string }) {
385481
}
386482
return <MissingOnDiskBadge path={absolutePath} compact />;
387483
}
484+
485+
/** Tiny inline sparkles icon to avoid importing full lucide for a single use */
486+
function SparklesIconInline() {
487+
return (
488+
<svg
489+
className="size-3"
490+
fill="none"
491+
stroke="currentColor"
492+
strokeLinecap="round"
493+
strokeLinejoin="round"
494+
strokeWidth={2}
495+
viewBox="0 0 24 24"
496+
>
497+
<path d="M9.937 15.5A2 2 0 0 0 8.5 14.063l-6.135-1.582a.5.5 0 0 1 0-.962L8.5 9.936A2 2 0 0 0 9.937 8.5l1.582-6.135a.5.5 0 0 1 .963 0L14.063 8.5A2 2 0 0 0 15.5 9.937l6.135 1.581a.5.5 0 0 1 0 .964L15.5 14.063a2 2 0 0 0-1.437 1.437l-1.582 6.135a.5.5 0 0 1-.963 0z" />
498+
<path d="M20 3v4" />
499+
<path d="M22 5h-4" />
500+
<path d="M4 17v2" />
501+
<path d="M5 18H3" />
502+
</svg>
503+
);
504+
}

0 commit comments

Comments
 (0)