From 62d187af34074f15ec40afe1ed603727f1825c0f Mon Sep 17 00:00:00 2001 From: Shy Alter Date: Wed, 29 Jul 2026 21:36:13 +0200 Subject: [PATCH 01/28] feat: add comments to task artifacts Add inline and document-level artifact comments with threads, image regions, resilient text anchors, sanitized HTML previews, version drift handling, and live query refresh. --- packages/api-client/src/posthog-client.ts | 115 +++++++ .../src/artifact-comments/anchors.test.ts | 65 ++++ .../core/src/artifact-comments/anchors.ts | 129 ++++++++ packages/core/src/sessions/sessionService.ts | 34 ++ .../components/DocumentPreviewHeader.tsx | 5 +- .../components/SelectionCommentOverlay.tsx | 16 +- .../components/CommentAnnotation.tsx | 8 +- .../components/AnnotatedArtifactImage.tsx | 209 ++++++++++++ .../components/ArtifactCommentsSidebar.tsx | 284 ++++++++++++++++ .../components/ArtifactPreview.test.tsx | 42 ++- .../sessions/components/ArtifactPreview.tsx | 312 +++++++++++++++--- .../components/ArtifactTextAnnotations.tsx | 256 ++++++++++++++ .../components/SanitizedArtifactHtml.test.ts | 41 +++ .../components/SanitizedArtifactHtml.tsx | 185 +++++++++++ .../components/useArtifactComments.ts | 105 ++++++ 15 files changed, 1758 insertions(+), 48 deletions(-) create mode 100644 packages/core/src/artifact-comments/anchors.test.ts create mode 100644 packages/core/src/artifact-comments/anchors.ts create mode 100644 packages/ui/src/features/sessions/components/AnnotatedArtifactImage.tsx create mode 100644 packages/ui/src/features/sessions/components/ArtifactCommentsSidebar.tsx create mode 100644 packages/ui/src/features/sessions/components/ArtifactTextAnnotations.tsx create mode 100644 packages/ui/src/features/sessions/components/SanitizedArtifactHtml.test.ts create mode 100644 packages/ui/src/features/sessions/components/SanitizedArtifactHtml.tsx create mode 100644 packages/ui/src/features/sessions/components/useArtifactComments.ts diff --git a/packages/api-client/src/posthog-client.ts b/packages/api-client/src/posthog-client.ts index 9a42697e08..c73956ee7f 100644 --- a/packages/api-client/src/posthog-client.ts +++ b/packages/api-client/src/posthog-client.ts @@ -179,6 +179,35 @@ export interface TaskSessionStorageAccess { content_sha256: string | null; } +export interface ArtifactCommentUser { + id: number; + uuid: string; + first_name?: string; + last_name?: string; + email: string; +} + +export interface ArtifactComment { + id: string; + created_by: ArtifactCommentUser | null; + content: string | null; + created_at: string; + deleted?: boolean | null; + item_id: string | null; + item_context: unknown; + scope: string; + source_comment: string | null; + is_task?: boolean; + completed_at?: string | null; +} + +export interface CreateArtifactCommentRequest { + artifactId: string; + content: string; + context: unknown; + sourceCommentId?: string; +} + /** Thrown when the backend rejects a cloud run with a 429 usage-limit error. */ export class CloudUsageLimitError extends Error { limitType: UsageLimitType; @@ -3293,6 +3322,92 @@ export class PostHogAPIClient { return data.url; } + async getArtifactComments(artifactId: string): Promise { + const teamId = await this.getTeamId(); + const comments: ArtifactComment[] = []; + let urlPath = `/api/projects/${teamId}/comments/?scope=task_artifact&item_id=${encodeURIComponent(artifactId)}`; + + // Comments use cursor pagination. Follow it so a long-running review never + // silently loses older threads, while retaining a defensive page ceiling. + for (let page = 0; page < 20; page++) { + const url = new URL(`${this.api.baseUrl}${urlPath}`); + const response = await this.api.fetcher.fetch({ + method: "get", + url, + path: url.pathname, + }); + if (!response.ok) { + throw new Error( + `Failed to fetch artifact comments: ${response.statusText}`, + ); + } + const data = (await response.json()) as { + results?: ArtifactComment[]; + next?: string | null; + }; + comments.push(...(data.results ?? [])); + if (!data.next) return comments; + const next = new URL(data.next); + urlPath = `${next.pathname}${next.search}`; + } + + log.warn("getArtifactComments hit the pagination limit", { artifactId }); + return comments; + } + + async createArtifactComment( + request: CreateArtifactCommentRequest, + ): Promise { + const teamId = await this.getTeamId(); + const path = `/api/projects/${teamId}/comments/`; + const url = new URL(`${this.api.baseUrl}${path}`); + const response = await this.api.fetcher.fetch({ + method: "post", + url, + path, + overrides: { + body: JSON.stringify({ + content: request.content, + scope: "task_artifact", + item_id: request.artifactId, + item_context: request.context, + source_comment: request.sourceCommentId ?? null, + // Root comments are actionable so the existing complete/reopen + // actions provide Google-Docs-style resolve semantics. + is_task: !request.sourceCommentId, + }), + }, + }); + if (!response.ok) { + throw new Error( + `Failed to create artifact comment: ${response.statusText}`, + ); + } + return (await response.json()) as ArtifactComment; + } + + async setArtifactCommentResolved( + commentId: string, + resolved: boolean, + ): Promise { + const teamId = await this.getTeamId(); + const action = resolved ? "complete" : "reopen"; + const path = `/api/projects/${teamId}/comments/${commentId}/${action}/`; + const url = new URL(`${this.api.baseUrl}${path}`); + const response = await this.api.fetcher.fetch({ + method: "post", + url, + path, + overrides: { body: "{}" }, + }); + if (!response.ok) { + throw new Error( + `Failed to ${action} artifact comment: ${response.statusText}`, + ); + } + return (await response.json()) as ArtifactComment; + } + async getTaskSessionStorageAccess( taskId: string, runId: string, diff --git a/packages/core/src/artifact-comments/anchors.test.ts b/packages/core/src/artifact-comments/anchors.test.ts new file mode 100644 index 0000000000..c3a2908f9f --- /dev/null +++ b/packages/core/src/artifact-comments/anchors.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { createTextArtifactAnchor, resolveTextArtifactAnchor } from "./anchors"; + +describe("artifact text anchors", () => { + it("creates and resolves a verified positional anchor", () => { + const text = "Before selected words after"; + const anchor = createTextArtifactAnchor(text, 7, 21); + + if (!anchor) throw new Error("Expected an anchor"); + expect(resolveTextArtifactAnchor(text, anchor)).toEqual({ + start: 7, + end: 21, + status: "exact", + }); + }); + + it("reanchors a quote after surrounding content changes", () => { + const original = "Before selected words after"; + const anchor = createTextArtifactAnchor(original, 7, 21); + if (!anchor) throw new Error("Expected an anchor"); + const changed = `New introduction. ${original}`; + + expect(resolveTextArtifactAnchor(changed, anchor)).toEqual({ + start: 25, + end: 39, + status: "reanchored", + }); + }); + + it("uses context to disambiguate repeated quotes", () => { + const original = "first repeated phrase then second repeated phrase end"; + const start = original.lastIndexOf("repeated phrase"); + const anchor = createTextArtifactAnchor( + original, + start, + start + "repeated phrase".length, + ); + if (!anchor) throw new Error("Expected an anchor"); + const changed = `prefix ${original}`; + + expect(resolveTextArtifactAnchor(changed, anchor)?.start).toBe( + changed.lastIndexOf("repeated phrase"), + ); + }); + + it("orphans deleted and ambiguous text instead of guessing", () => { + const deleted = createTextArtifactAnchor("unique text", 0, 6); + if (!deleted) throw new Error("Expected an anchor"); + expect(resolveTextArtifactAnchor("replacement", deleted)).toBeNull(); + + const ambiguous = { + kind: "text" as const, + quote: "same", + prefix: "", + suffix: "", + start: 100, + end: 104, + }; + expect(resolveTextArtifactAnchor("same x same", ambiguous)).toBeNull(); + }); + + it("rejects whitespace-only selections", () => { + expect(createTextArtifactAnchor("a b", 1, 4)).toBeNull(); + }); +}); diff --git a/packages/core/src/artifact-comments/anchors.ts b/packages/core/src/artifact-comments/anchors.ts new file mode 100644 index 0000000000..ba1cb11630 --- /dev/null +++ b/packages/core/src/artifact-comments/anchors.ts @@ -0,0 +1,129 @@ +import { z } from "zod"; + +const CONTEXT_LENGTH = 32; + +export const textArtifactAnchorSchema = z.object({ + kind: z.literal("text"), + quote: z.string().min(1), + prefix: z.string(), + suffix: z.string(), + start: z.number().int().nonnegative(), + end: z.number().int().positive(), +}); + +export const regionArtifactAnchorSchema = z.object({ + kind: z.literal("region"), + x: z.number().min(0).max(1), + y: z.number().min(0).max(1), + width: z.number().min(0).max(1), + height: z.number().min(0).max(1), +}); + +export const documentArtifactAnchorSchema = z.object({ + kind: z.literal("document"), +}); + +export const artifactAnchorSchema = z.discriminatedUnion("kind", [ + textArtifactAnchorSchema, + regionArtifactAnchorSchema, + documentArtifactAnchorSchema, +]); + +export type TextArtifactAnchor = z.infer; +export type RegionArtifactAnchor = z.infer; +export type ArtifactAnchor = z.infer; + +export const artifactCommentContextSchema = z.object({ + taskId: z.string(), + runId: z.string(), + artifactId: z.string(), + artifactVersion: z.string(), + anchor: artifactAnchorSchema, +}); + +export type ArtifactCommentContext = z.infer< + typeof artifactCommentContextSchema +>; + +export type ResolvedTextAnchor = { + start: number; + end: number; + status: "exact" | "reanchored"; +}; + +export function createTextArtifactAnchor( + text: string, + start: number, + end: number, +): TextArtifactAnchor | null { + const safeStart = Math.max(0, Math.min(start, text.length)); + const safeEnd = Math.max(safeStart, Math.min(end, text.length)); + const quote = text.slice(safeStart, safeEnd); + if (!quote.trim()) return null; + + return { + kind: "text", + quote, + prefix: text.slice(Math.max(0, safeStart - CONTEXT_LENGTH), safeStart), + suffix: text.slice(safeEnd, safeEnd + CONTEXT_LENGTH), + start: safeStart, + end: safeEnd, + }; +} + +function candidateScore( + text: string, + start: number, + anchor: TextArtifactAnchor, +): number { + const prefix = text.slice(Math.max(0, start - anchor.prefix.length), start); + const end = start + anchor.quote.length; + const suffix = text.slice(end, end + anchor.suffix.length); + let score = 0; + if (anchor.prefix && prefix === anchor.prefix) score += 2; + if (anchor.suffix && suffix === anchor.suffix) score += 2; + return score; +} + +/** + * Resolve a persisted text quote without ever guessing. The stored position is + * verified first. If content moved, prefix/suffix disambiguate quote matches; + * ties are deliberately treated as orphaned. + */ +export function resolveTextArtifactAnchor( + text: string, + anchor: TextArtifactAnchor, +): ResolvedTextAnchor | null { + if (text.slice(anchor.start, anchor.end) === anchor.quote) { + return { start: anchor.start, end: anchor.end, status: "exact" }; + } + + const candidates: number[] = []; + let from = 0; + while (from <= text.length - anchor.quote.length) { + const match = text.indexOf(anchor.quote, from); + if (match < 0) break; + candidates.push(match); + from = match + Math.max(anchor.quote.length, 1); + } + if (candidates.length === 0) return null; + if (candidates.length === 1) { + const start = candidates[0]; + return { + start, + end: start + anchor.quote.length, + status: "reanchored", + }; + } + + const ranked = candidates + .map((start) => ({ start, score: candidateScore(text, start, anchor) })) + .sort((a, b) => b.score - a.score); + if (ranked[0].score === 0 || ranked[0].score === ranked[1].score) return null; + + return { + start: ranked[0].start, + end: ranked[0].start + anchor.quote.length, + status: "reanchored", + }; +} diff --git a/packages/core/src/sessions/sessionService.ts b/packages/core/src/sessions/sessionService.ts index cd001a33e5..1df5ee5a05 100644 --- a/packages/core/src/sessions/sessionService.ts +++ b/packages/core/src/sessions/sessionService.ts @@ -9,6 +9,10 @@ import type { SessionConfigSelectOption, SessionUpdate, } from "@agentclientprotocol/sdk"; +import type { + ArtifactComment, + CreateArtifactCommentRequest, +} from "@posthog/api-client/posthog-client"; import { type AcpMessage, type Adapter, @@ -7406,6 +7410,36 @@ export class SessionService { } } + async getArtifactComments(artifactId: string): Promise { + const authStatus = await this.getAuthCredentialsStatus(); + if (authStatus.kind !== "ready") return []; + return authStatus.auth.client.getArtifactComments(artifactId); + } + + async createArtifactComment( + request: CreateArtifactCommentRequest, + ): Promise { + const authStatus = await this.getAuthCredentialsStatus(); + if (authStatus.kind !== "ready") { + throw new Error("Sign in to comment on artifacts"); + } + return authStatus.auth.client.createArtifactComment(request); + } + + async setArtifactCommentResolved( + commentId: string, + resolved: boolean, + ): Promise { + const authStatus = await this.getAuthCredentialsStatus(); + if (authStatus.kind !== "ready") { + throw new Error("Sign in to resolve artifact comments"); + } + return authStatus.auth.client.setArtifactCommentResolved( + commentId, + resolved, + ); + } + async getCloudRunArtifacts( taskId: string, runId: string, diff --git a/packages/ui/src/features/code-editor/components/DocumentPreviewHeader.tsx b/packages/ui/src/features/code-editor/components/DocumentPreviewHeader.tsx index 8a336f8914..2948dada95 100644 --- a/packages/ui/src/features/code-editor/components/DocumentPreviewHeader.tsx +++ b/packages/ui/src/features/code-editor/components/DocumentPreviewHeader.tsx @@ -1,6 +1,6 @@ import { Check, Code, Copy, Eye } from "@phosphor-icons/react"; import { Flex, IconButton, Text } from "@radix-ui/themes"; -import { useState } from "react"; +import { type ReactNode, useState } from "react"; import { Tooltip } from "../../../primitives/Tooltip"; export function DocumentPreviewHeader({ @@ -8,11 +8,13 @@ export function DocumentPreviewHeader({ content, showRendered, onToggleRendered, + actions, }: { label: string; content: string; showRendered: boolean; onToggleRendered: () => void; + actions?: ReactNode; }) { const [copied, setCopied] = useState(false); @@ -34,6 +36,7 @@ export function DocumentPreviewHeader({ {label} + {actions} void; onDismiss: () => void; + actionLabel?: string; + placeholder?: string; } /** @@ -37,6 +39,8 @@ export function SelectionCommentOverlay({ filePath, onSubmit, onDismiss, + actionLabel = "Add to chat", + placeholder, }: SelectionCommentOverlayProps) { if (!open || !selection?.anchor) return null; // Key by the range so a fresh selection remounts the card back to the "+". @@ -49,6 +53,8 @@ export function SelectionCommentOverlay({ filePath={filePath} onSubmit={onSubmit} onDismiss={onDismiss} + actionLabel={actionLabel} + placeholder={placeholder} /> ); } @@ -60,6 +66,8 @@ function SelectionComposerCard({ filePath, onSubmit, onDismiss, + actionLabel, + placeholder, }: { anchor: { top: number; left: number }; fromLine: number; @@ -67,18 +75,20 @@ function SelectionComposerCard({ filePath: string; onSubmit: (startLine: number, endLine: number, text: string) => void; onDismiss: () => void; + actionLabel: string; + placeholder?: string; }) { const [expanded, setExpanded] = useState(false); const style = { top: anchor.top + 4, left: anchor.left }; if (!expanded) { return createPortal( - + + {imageBox && ( +
{ + if (commenting && (event.key === "Enter" || event.key === " ")) { + event.preventDefault(); + const box = event.currentTarget.getBoundingClientRect(); + openComposer( + 0.5, + 0.5, + box.left + box.width / 2, + box.top + box.height / 2, + ); + } + }} + onClick={(event) => { + if (!commenting) return; + const box = event.currentTarget.getBoundingClientRect(); + openComposer( + (event.clientX - box.left) / box.width, + (event.clientY - box.top) / box.height, + event.clientX, + event.clientY, + ); + }} + > + {regionComments.map(({ comment, anchor }) => ( +
+ )} + { + if (pendingAnchor) onCreate(pendingAnchor, content); + dismiss(); + }} + /> + + ); +} diff --git a/packages/ui/src/features/sessions/components/ArtifactCommentsSidebar.tsx b/packages/ui/src/features/sessions/components/ArtifactCommentsSidebar.tsx new file mode 100644 index 0000000000..58f42cf078 --- /dev/null +++ b/packages/ui/src/features/sessions/components/ArtifactCommentsSidebar.tsx @@ -0,0 +1,284 @@ +import { + ArrowCounterClockwise, + ChatCircle, + CheckCircle, + WarningCircle, + X, +} from "@phosphor-icons/react"; +import type { ArtifactComment } from "@posthog/api-client/posthog-client"; +import { + Avatar, + AvatarFallback, + Button, + Spinner, + Textarea, +} from "@posthog/quill"; +import { formatRelativeTimeShort } from "@posthog/shared"; +import { useMemo, useState } from "react"; +import { + type HighlightResolution, + parseArtifactCommentContext, +} from "./ArtifactTextAnnotations"; + +function authorName(comment: ArtifactComment): string { + const user = comment.created_by; + if (!user) return "You"; + return ( + [user.first_name, user.last_name].filter(Boolean).join(" ") || user.email + ); +} + +function initials(name: string): string { + return name + .split(/\s+/) + .slice(0, 2) + .map((part) => part[0]?.toUpperCase()) + .join(""); +} + +function CommentBody({ comment }: { comment: ArtifactComment }) { + const name = authorName(comment); + return ( +
+ + {initials(name) || "?"} + +
+
+ {name} + + {formatRelativeTimeShort(comment.created_at)} + +
+

+ {comment.content} +

+
+
+ ); +} + +function Thread({ + root, + replies, + selected, + currentVersion, + resolution, + busy, + onSelect, + onReply, + onResolve, +}: { + root: ArtifactComment; + replies: ArtifactComment[]; + selected: boolean; + currentVersion: string; + resolution?: HighlightResolution; + busy: boolean; + onSelect: () => void; + onReply: (content: string) => void; + onResolve: (resolved: boolean) => void; +}) { + const [replying, setReplying] = useState(false); + const [reply, setReply] = useState(""); + const resolved = !!root.completed_at; + const context = parseArtifactCommentContext(root); + const anotherVersion = + context?.artifactVersion && context.artifactVersion !== currentVersion; + + return ( +
+ + {replies.length > 0 && ( +
+ {replies.map((comment) => ( + + ))} +
+ )} + {replying ? ( +
+