From 4b99424c5fe3e6ac2cbf7767e8abced9187575e4 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Sun, 29 Mar 2026 01:47:07 -0500 Subject: [PATCH] Reorganize conflict intake UI and expandable summaries - Extract shared merge-conflict logic into reusable helpers - Add expandable summary preview sheets for long guidance text - Refresh intake layout with tooltip help and improved workspace display --- .../ExpandableSummary.test.tsx | 59 ++++++ .../merge-conflicts/ExpandableSummary.tsx | 89 +++++++++ .../MergeConflictShell.logic.test.ts | 53 ++++++ .../MergeConflictShell.logic.ts | 22 +++ .../merge-conflicts/MergeConflictShell.tsx | 174 ++++++++++-------- .../src/components/review/reviewUtils.test.ts | 63 +++++++ apps/web/src/index.css | 121 ++++++++++++ 7 files changed, 502 insertions(+), 79 deletions(-) create mode 100644 apps/web/src/components/merge-conflicts/ExpandableSummary.test.tsx create mode 100644 apps/web/src/components/merge-conflicts/ExpandableSummary.tsx create mode 100644 apps/web/src/components/review/reviewUtils.test.ts diff --git a/apps/web/src/components/merge-conflicts/ExpandableSummary.test.tsx b/apps/web/src/components/merge-conflicts/ExpandableSummary.test.tsx new file mode 100644 index 00000000..3074e836 --- /dev/null +++ b/apps/web/src/components/merge-conflicts/ExpandableSummary.test.tsx @@ -0,0 +1,59 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vitest"; + +import { ExpandableSummary } from "./ExpandableSummary"; + +describe("ExpandableSummary", () => { + it("renders inline text by default when no children are provided", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain( + "This is an AI summary that is long enough to be expandable and interesting.", + ); + }); + + it("renders custom children when provided", () => { + const html = renderToStaticMarkup( + + Custom inline render + , + ); + expect(html).toContain("Custom inline render"); + expect(html).toContain("custom"); + }); + + it("shows the expand button for text longer than 40 characters", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain("Expand summary"); + }); + + it("hides the expand button for very short text", () => { + const html = renderToStaticMarkup(); + expect(html).not.toContain("Expand summary"); + }); + + it("hides the expand button for whitespace-only text", () => { + const html = renderToStaticMarkup(); + expect(html).not.toContain("Expand summary"); + }); + + it("applies the group/expand class for hover interaction", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain("group/expand"); + }); + + it("passes className to the wrapper div", () => { + const html = renderToStaticMarkup( + , + ); + expect(html).toContain("mt-3"); + }); +}); diff --git a/apps/web/src/components/merge-conflicts/ExpandableSummary.tsx b/apps/web/src/components/merge-conflicts/ExpandableSummary.tsx new file mode 100644 index 00000000..7eaf4a01 --- /dev/null +++ b/apps/web/src/components/merge-conflicts/ExpandableSummary.tsx @@ -0,0 +1,89 @@ +import { MaximizeIcon } from "lucide-react"; +import { memo, useState } from "react"; +import ReactMarkdown from "react-markdown"; +import remarkGfm from "remark-gfm"; + +import { cn } from "~/lib/utils"; +import { Sheet, SheetPopup, SheetPanel } from "~/components/ui/sheet"; + +/** + * Wraps plain-text or markdown AI summaries with an expand button + * that opens a clean, Notion-like full-height preview sheet. + * + * Usage: + * + * + * The inline render is plain text (default children or the `text` prop). + * The expanded view renders the text as GitHub-flavored Markdown in a + * minimal, readable layout. + */ + +interface ExpandableSummaryProps { + /** The raw text / markdown content to render */ + text: string; + /** Title shown at the top of the expanded sheet */ + title?: string; + /** Optional subtitle / context line below the title */ + subtitle?: string; + /** Additional className for the inline wrapper */ + className?: string; + /** Inline text element — defaults to rendering `text` in a

*/ + children?: React.ReactNode; +} + +export const ExpandableSummary = memo(function ExpandableSummary({ + text, + title, + subtitle, + className, + children, +}: ExpandableSummaryProps) { + const [open, setOpen] = useState(false); + + // Don't render the expand affordance for very short text + const isExpandable = text.trim().length > 40; + + return ( + <> +

+ {children ??

{text}

} + + {isExpandable ? ( + + ) : null} +
+ + {isExpandable ? ( + + + +
+ {title ? ( +
+

+ {title} +

+ {subtitle ? ( +

{subtitle}

+ ) : null} +
+ ) : null} + +
+ {text} +
+
+ + + + ) : null} + + ); +}); diff --git a/apps/web/src/components/merge-conflicts/MergeConflictShell.logic.test.ts b/apps/web/src/components/merge-conflicts/MergeConflictShell.logic.test.ts index 3d98e164..4cc64bdd 100644 --- a/apps/web/src/components/merge-conflicts/MergeConflictShell.logic.test.ts +++ b/apps/web/src/components/merge-conflicts/MergeConflictShell.logic.test.ts @@ -7,6 +7,8 @@ import { groupConflictCandidatesByFile, humanizeConflictError, pickRecommendedConflictCandidate, + pullRequestStateBadgeClassName, + workspaceModeLabel, } from "./MergeConflictShell.logic"; describe("pickRecommendedConflictCandidate", () => { @@ -211,3 +213,54 @@ describe("buildConflictFeedbackPreview", () => { ).toContain("Operator note: Keep the API signature from the incoming branch."); }); }); + +describe("workspaceModeLabel", () => { + it('returns "Repo scan" when no workspace is prepared', () => { + expect(workspaceModeLabel(null)).toBe("Repo scan"); + }); + + it('returns "Prepared in repo" for local mode', () => { + expect( + workspaceModeLabel({ + branch: "feature/auth", + cwd: "/Users/val/project", + mode: "local", + worktreePath: null, + }), + ).toBe("Prepared in repo"); + }); + + it('returns "Dedicated worktree" for worktree mode', () => { + expect( + workspaceModeLabel({ + branch: "feature/auth", + cwd: "/Users/val/.git/worktrees/auth", + mode: "worktree", + worktreePath: "/Users/val/.git/worktrees/auth", + }), + ).toBe("Dedicated worktree"); + }); +}); + +describe("pullRequestStateBadgeClassName", () => { + it("returns emerald styling for open PRs", () => { + const result = pullRequestStateBadgeClassName("open"); + expect(result).toContain("emerald"); + expect(result).not.toContain("muted"); + }); + + it("returns muted styling for merged PRs", () => { + const result = pullRequestStateBadgeClassName("merged"); + expect(result).toContain("muted"); + expect(result).not.toContain("emerald"); + }); + + it("returns muted styling for closed PRs", () => { + const result = pullRequestStateBadgeClassName("closed"); + expect(result).toContain("muted"); + }); + + it("returns identical styling for merged and closed states", () => { + expect(pullRequestStateBadgeClassName("merged")).toBe(pullRequestStateBadgeClassName("closed")); + }); +}); diff --git a/apps/web/src/components/merge-conflicts/MergeConflictShell.logic.ts b/apps/web/src/components/merge-conflicts/MergeConflictShell.logic.ts index 8fd693ea..d9c1ce1b 100644 --- a/apps/web/src/components/merge-conflicts/MergeConflictShell.logic.ts +++ b/apps/web/src/components/merge-conflicts/MergeConflictShell.logic.ts @@ -224,3 +224,25 @@ export function computeActiveStepIndex( const index = steps.findIndex((step) => step.status !== "done"); return index === -1 ? steps.length : index; } + +export interface PreparedWorkspace { + branch: string; + cwd: string; + mode: "local" | "worktree"; + worktreePath: string | null; +} + +export function workspaceModeLabel(workspace: PreparedWorkspace | null): string { + if (!workspace) return "Repo scan"; + return workspace.mode === "worktree" ? "Dedicated worktree" : "Prepared in repo"; +} + +export function pullRequestStateBadgeClassName(state: GitResolvedPullRequest["state"]): string { + switch (state) { + case "open": + return "border-emerald-500/30 bg-emerald-500/12 text-emerald-700 dark:text-emerald-300"; + case "merged": + case "closed": + return "border-border bg-muted/70 text-foreground"; + } +} diff --git a/apps/web/src/components/merge-conflicts/MergeConflictShell.tsx b/apps/web/src/components/merge-conflicts/MergeConflictShell.tsx index d3f20264..f8bdd8e9 100644 --- a/apps/web/src/components/merge-conflicts/MergeConflictShell.tsx +++ b/apps/web/src/components/merge-conflicts/MergeConflictShell.tsx @@ -53,13 +53,8 @@ import { } from "~/components/ui/empty"; import { Input } from "~/components/ui/input"; import { ScrollArea } from "~/components/ui/scroll-area"; -import { - Select, - SelectItem, - SelectPopup, - SelectTrigger, - SelectValue, -} from "~/components/ui/select"; +import { Select, SelectButton, SelectItem, SelectPopup } from "~/components/ui/select"; +import { Tooltip, TooltipTrigger, TooltipPopup } from "~/components/ui/tooltip"; import { Sheet, SheetDescription, @@ -80,8 +75,12 @@ import { groupConflictCandidatesByFile, humanizeConflictError, type MergeConflictFeedbackDisposition, + type PreparedWorkspace, pickRecommendedConflictCandidate, + pullRequestStateBadgeClassName, + workspaceModeLabel, } from "./MergeConflictShell.logic"; +import { ExpandableSummary } from "./ExpandableSummary"; const FEEDBACK_DISPOSITION_SCHEMA = Schema.Literals(["accept", "review", "escalate", "blocked"]); const FEEDBACK_DRAFT_SCHEMA = Schema.Struct({ @@ -106,23 +105,6 @@ const FEEDBACK_DISPOSITION_OPTIONS: ReadonlyArray<{ { value: "blocked", label: "Blocked" }, ] as const; -interface PreparedWorkspace { - branch: string; - cwd: string; - mode: "local" | "worktree"; - worktreePath: string | null; -} - -function pullRequestStateBadgeClassName(state: GitResolvedPullRequest["state"]) { - switch (state) { - case "open": - return "border-emerald-500/30 bg-emerald-500/12 text-emerald-700 dark:text-emerald-300"; - case "merged": - case "closed": - return "border-border bg-muted/70 text-foreground"; - } -} - function tonePanelClassName(tone: "neutral" | "success" | "warning") { switch (tone) { case "success": @@ -147,11 +129,6 @@ function stepStatusClassName(status: "done" | "active" | "todo" | "blocked") { } } -function workspaceModeLabel(workspace: PreparedWorkspace | null): string { - if (!workspace) return "Repo scan"; - return workspace.mode === "worktree" ? "Dedicated worktree" : "Prepared in repo"; -} - async function openPathInEditor(targetPath: string) { await openInPreferredEditor(ensureNativeApi(), targetPath); } @@ -198,7 +175,14 @@ function ConflictCandidateButton({ {candidate.confidence} -

{candidate.description}

+ +

{candidate.description}

+ ); } @@ -679,37 +663,49 @@ export function MergeConflictShell({
+ + + + + Resolve conflicts from a GitHub PR link, then let OK Code guide the safest + next action. + + + } />
-
-
-

- Agent note -

-

- {selectedCandidate - ? selectedCandidate.confidence === "safe" - ? "This candidate is deterministic. Review the resulting diff after apply, but it is the lowest-risk path OK Code could infer." - : "This candidate is only a starting point. Keep the handoff note explicit and verify the merged intent before you commit." - : "No candidate is selected. OK Code will not mutate the workspace until you review a concrete patch."} -

-
+ {(() => { + const agentNote = selectedCandidate + ? selectedCandidate.confidence === "safe" + ? "This candidate is deterministic. Review the resulting diff after apply, but it is the lowest-risk path OK Code could infer." + : "This candidate is only a starting point. Keep the handoff note explicit and verify the merged intent before you commit." + : "No candidate is selected. OK Code will not mutate the workspace until you review a concrete patch."; + return ( +
+

+ Agent note +

+ +

+ {agentNote} +

+ +
+ ); + })()}
diff --git a/apps/web/src/components/review/reviewUtils.test.ts b/apps/web/src/components/review/reviewUtils.test.ts new file mode 100644 index 00000000..c031cf93 --- /dev/null +++ b/apps/web/src/components/review/reviewUtils.test.ts @@ -0,0 +1,63 @@ +import type { ProjectId } from "@okcode/contracts"; +import { describe, expect, it } from "vitest"; + +import { joinPath, projectLabel } from "./reviewUtils"; + +function makeProject(overrides: { id?: string; name: string; cwd: string }) { + return { + id: (overrides.id ?? "test-id") as ProjectId, + name: overrides.name, + cwd: overrides.cwd, + model: "claude-opus-4", + expanded: false, + scripts: [], + }; +} + +describe("projectLabel", () => { + it("returns the project name when present", () => { + expect( + projectLabel( + makeProject({ name: "okcode", cwd: "/Users/val/Documents/GitHub/OpenKnots/okcode" }), + ), + ).toBe("okcode"); + }); + + it("falls back to cwd when name is empty", () => { + expect(projectLabel(makeProject({ name: "", cwd: "/Users/val/projects/demo" }))).toBe( + "/Users/val/projects/demo", + ); + }); + + it("falls back to cwd when name is only whitespace", () => { + expect(projectLabel(makeProject({ name: " ", cwd: "/opt/repos/app" }))).toBe( + "/opt/repos/app", + ); + }); + + it("preserves the original name without trimming", () => { + expect(projectLabel(makeProject({ name: " my-project ", cwd: "/tmp" }))).toBe(" my-project "); + }); +}); + +describe("joinPath", () => { + it("joins base and relative path", () => { + expect(joinPath("/Users/val/project", "src/index.ts")).toBe("/Users/val/project/src/index.ts"); + }); + + it("strips trailing slashes from base", () => { + expect(joinPath("/Users/val/project///", "src/index.ts")).toBe( + "/Users/val/project/src/index.ts", + ); + }); + + it("strips leading slashes from relative path", () => { + expect(joinPath("/Users/val/project", "///src/index.ts")).toBe( + "/Users/val/project/src/index.ts", + ); + }); + + it("handles both trailing and leading slashes", () => { + expect(joinPath("/base/", "/relative")).toBe("/base/relative"); + }); +}); diff --git a/apps/web/src/index.css b/apps/web/src/index.css index da1a706c..85c306c3 100644 --- a/apps/web/src/index.css +++ b/apps/web/src/index.css @@ -394,6 +394,127 @@ label:has(> select#reasoning-effort) select { text-align: left; } +/* ── Summary preview (Notion-like expanded markdown) ─────────── */ +.summary-preview-body > :first-child { + margin-top: 0; +} + +.summary-preview-body > :last-child { + margin-bottom: 0; +} + +.summary-preview-body p, +.summary-preview-body ul, +.summary-preview-body ol, +.summary-preview-body blockquote, +.summary-preview-body pre, +.summary-preview-body table { + margin: 0.85rem 0; +} + +.summary-preview-body h1, +.summary-preview-body h2, +.summary-preview-body h3 { + margin-top: 1.6rem; + margin-bottom: 0.45rem; + font-weight: 600; + line-height: 1.35; + color: var(--foreground); +} + +.summary-preview-body h1 { + font-size: 1.375rem; +} + +.summary-preview-body h2 { + font-size: 1.15rem; +} + +.summary-preview-body h3 { + font-size: 1rem; +} + +.summary-preview-body ul { + padding-left: 1.35rem; + list-style-type: disc; +} + +.summary-preview-body ol { + padding-left: 1.35rem; + list-style-type: decimal; +} + +.summary-preview-body li + li { + margin-top: 0.35rem; +} + +.summary-preview-body li > p { + margin: 0.25rem 0; +} + +.summary-preview-body a { + color: var(--info-foreground); + text-decoration: underline; + text-underline-offset: 2px; +} + +.summary-preview-body blockquote { + border-left: 3px solid var(--border); + padding-left: 1rem; + color: var(--muted-foreground); + font-style: italic; +} + +.summary-preview-body :not(pre) > code { + border: 1px solid var(--border); + border-radius: 0.375rem; + padding: 0.15em 0.35em; + font-size: 0.85em; + background: color-mix(in srgb, var(--muted) 60%, transparent); +} + +.summary-preview-body pre { + max-width: 100%; + overflow-x: auto; + border: 1px solid var(--border); + border-radius: 0.625rem; + padding: 0.875rem 1rem; + background: color-mix(in srgb, var(--muted) 50%, var(--background)); + font-size: 0.8125rem; + line-height: 1.65; +} + +.summary-preview-body pre code { + border: none; + background: transparent; + padding: 0; + font-size: inherit; +} + +.summary-preview-body table { + width: 100%; + border-collapse: collapse; + font-size: 0.875rem; +} + +.summary-preview-body th, +.summary-preview-body td { + border: 1px solid var(--border); + padding: 0.45rem 0.65rem; + text-align: left; +} + +.summary-preview-body th { + font-weight: 600; + background: color-mix(in srgb, var(--muted) 40%, transparent); +} + +.summary-preview-body hr { + border: none; + border-top: 1px solid var(--border); + margin: 1.5rem 0; +} + /* Diffs theme bridge (match diff surfaces to app palette) */ .diff-panel-viewport { background: color-mix(in srgb, var(--background) 94%, var(--card));