From c1ef934ecf602cdb1ec14240bc8c0e3e21475382 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:08:36 -0700 Subject: [PATCH 01/30] feat(PRO-546): add local comment wire types --- packages/types/package.json | 1 + packages/types/src/comments.ts | 60 ++++++++++++++++++++++++++++++++++ packages/types/src/index.ts | 1 + 3 files changed, 62 insertions(+) create mode 100644 packages/types/src/comments.ts diff --git a/packages/types/package.json b/packages/types/package.json index a2260c4..3781db0 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -7,6 +7,7 @@ "exports": { ".": "./src/index.ts", "./chapters": "./src/chapters.ts", + "./comments": "./src/comments.ts", "./diff": "./src/diff.ts", "./parsed-diff": "./src/parsed-diff.ts", "./prologue": "./src/prologue.ts", diff --git a/packages/types/src/comments.ts b/packages/types/src/comments.ts new file mode 100644 index 0000000..845a2d9 --- /dev/null +++ b/packages/types/src/comments.ts @@ -0,0 +1,60 @@ +import { z } from "zod"; +import { DIFF_SIDE } from "./chapters.ts"; + +// A single authored comment. Replies are sibling comments sharing a thread, so a +// comment carries no positional data of its own — the thread owns the anchor. +// Non-strict (like the other wire response schemas) so the server can add fields +// the SPA doesn't yet read without the response failing to parse. +export const CommentSchema = z.object({ + id: z.string(), + body: z.string(), + authorId: z.string(), + createdAt: z.string(), + updatedAt: z.string(), +}); +export type Comment = z.infer; + +// A line-anchored conversation. `comments` is ordered oldest-first; the first is +// the thread's root. `resolvedAt` is null while the thread is open. +export const CommentThreadSchema = z.object({ + id: z.string(), + filePath: z.string(), + side: z.enum(DIFF_SIDE), + startLine: z.number().int().positive(), + endLine: z.number().int().positive(), + resolvedAt: z.string().nullable(), + createdAt: z.string(), + updatedAt: z.string(), + comments: z.array(CommentSchema), +}); +export type CommentThread = z.infer; + +export const CommentThreadsResponseSchema = z.array(CommentThreadSchema); +export type CommentThreadsResponse = z.infer; + +// Body for creating a thread + its root comment in one request. +export const CreateCommentThreadBodySchema = z + .object({ + filePath: z.string().min(1), + side: z.enum(DIFF_SIDE), + startLine: z.number().int().positive(), + endLine: z.number().int().positive(), + body: z.string().min(1), + }) + .refine((v) => v.startLine <= v.endLine, { + message: "endLine must be greater than or equal to startLine", + path: ["endLine"], + }); +export type CreateCommentThreadBody = z.infer; + +// Body for adding a reply or editing an existing comment. +export const CommentBodySchema = z.object({ + body: z.string().min(1), +}); +export type CommentBody = z.infer; + +// Body for toggling a thread's resolved state. +export const ResolveThreadBodySchema = z.object({ + resolved: z.boolean(), +}); +export type ResolveThreadBody = z.infer; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 715cf6b..4aa846d 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -1,4 +1,5 @@ export * from "./chapters.ts"; +export * from "./comments.ts"; export * from "./diff.ts"; export * from "./parsed-diff.ts"; export * from "./prologue.ts"; From ae3bced82e689cb3deab1a16a1c0c54467e1f8ec Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:11:42 -0700 Subject: [PATCH 02/30] feat(PRO-546): add comment_thread + comment tables, extract deriveScopeKey --- packages/cli/drizzle/0006_puzzling_beast.sql | 24 + packages/cli/drizzle/meta/0006_snapshot.json | 775 +++++++++++++++++++ packages/cli/drizzle/meta/_journal.json | 7 + packages/cli/src/db/schema/comment-thread.ts | 23 + packages/cli/src/db/schema/comment.ts | 20 + packages/cli/src/db/schema/index.ts | 2 + packages/cli/src/runs/import-chapters.ts | 43 +- packages/cli/src/runs/scope-key.ts | 24 + 8 files changed, 893 insertions(+), 25 deletions(-) create mode 100644 packages/cli/drizzle/0006_puzzling_beast.sql create mode 100644 packages/cli/drizzle/meta/0006_snapshot.json create mode 100644 packages/cli/src/db/schema/comment-thread.ts create mode 100644 packages/cli/src/db/schema/comment.ts create mode 100644 packages/cli/src/runs/scope-key.ts diff --git a/packages/cli/drizzle/0006_puzzling_beast.sql b/packages/cli/drizzle/0006_puzzling_beast.sql new file mode 100644 index 0000000..e1242c3 --- /dev/null +++ b/packages/cli/drizzle/0006_puzzling_beast.sql @@ -0,0 +1,24 @@ +CREATE TABLE `comment` ( + `id` text PRIMARY KEY NOT NULL, + `createdAt` integer NOT NULL, + `updatedAt` integer NOT NULL, + `threadId` text NOT NULL, + `authorId` text DEFAULT 'local' NOT NULL, + `body` text NOT NULL, + FOREIGN KEY (`threadId`) REFERENCES `comment_thread`(`id`) ON UPDATE no action ON DELETE cascade +); +--> statement-breakpoint +CREATE INDEX `comment_thread_id_idx` ON `comment` (`threadId`);--> statement-breakpoint +CREATE TABLE `comment_thread` ( + `id` text PRIMARY KEY NOT NULL, + `createdAt` integer NOT NULL, + `updatedAt` integer NOT NULL, + `scopeKey` text NOT NULL, + `filePath` text NOT NULL, + `side` text NOT NULL, + `startLine` integer NOT NULL, + `endLine` integer NOT NULL, + `resolvedAt` integer +); +--> statement-breakpoint +CREATE INDEX `comment_thread_scope_key_idx` ON `comment_thread` (`scopeKey`); \ No newline at end of file diff --git a/packages/cli/drizzle/meta/0006_snapshot.json b/packages/cli/drizzle/meta/0006_snapshot.json new file mode 100644 index 0000000..58ba90c --- /dev/null +++ b/packages/cli/drizzle/meta/0006_snapshot.json @@ -0,0 +1,775 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "a44dabe5-4d9e-445a-994d-fa87195fc4b2", + "prevId": "eba48091-b53e-48b7-b8df-6229e6f4e2ee", + "tables": { + "chapter": { + "name": "chapter", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "runId": { + "name": "runId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "externalId": { + "name": "externalId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "chapterIndex": { + "name": "chapterIndex", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "summary": { + "name": "summary", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "hunkRefs": { + "name": "hunkRefs", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "keyChanges": { + "name": "keyChanges", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + } + }, + "indexes": { + "chapter_run_idx_unique": { + "name": "chapter_run_idx_unique", + "columns": [ + "runId", + "chapterIndex" + ], + "isUnique": true + } + }, + "foreignKeys": { + "chapter_runId_chapter_run_id_fk": { + "name": "chapter_runId_chapter_run_id_fk", + "tableFrom": "chapter", + "tableTo": "chapter_run", + "columnsFrom": [ + "runId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "chapter_file_view": { + "name": "chapter_file_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "chapterId": { + "name": "chapterId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filePath": { + "name": "filePath", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "chapter_file_view_chapter_id_idx": { + "name": "chapter_file_view_chapter_id_idx", + "columns": [ + "chapterId" + ], + "isUnique": false + }, + "chapter_file_view_user_chapter_path_unique": { + "name": "chapter_file_view_user_chapter_path_unique", + "columns": [ + "userId", + "chapterId", + "filePath" + ], + "isUnique": true + } + }, + "foreignKeys": { + "chapter_file_view_chapterId_chapter_id_fk": { + "name": "chapter_file_view_chapterId_chapter_id_fk", + "tableFrom": "chapter_file_view", + "tableTo": "chapter", + "columnsFrom": [ + "chapterId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "chapter_run": { + "name": "chapter_run", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "repoRoot": { + "name": "repoRoot", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "originUrl": { + "name": "originUrl", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "prNumber": { + "name": "prNumber", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "scopeKind": { + "name": "scopeKind", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "workingTreeRef": { + "name": "workingTreeRef", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "baseSha": { + "name": "baseSha", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "headSha": { + "name": "headSha", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "mergeBaseSha": { + "name": "mergeBaseSha", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "generatedAt": { + "name": "generatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "prologue": { + "name": "prologue", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "chapter_run_created_at_idx": { + "name": "chapter_run_created_at_idx", + "columns": [ + "createdAt" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "chapter_view": { + "name": "chapter_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "chapterId": { + "name": "chapterId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "chapter_view_user_chapter_unique": { + "name": "chapter_view_user_chapter_unique", + "columns": [ + "userId", + "chapterId" + ], + "isUnique": true + } + }, + "foreignKeys": { + "chapter_view_chapterId_chapter_id_fk": { + "name": "chapter_view_chapterId_chapter_id_fk", + "tableFrom": "chapter_view", + "tableTo": "chapter", + "columnsFrom": [ + "chapterId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "comment": { + "name": "comment", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "threadId": { + "name": "threadId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "authorId": { + "name": "authorId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "body": { + "name": "body", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "comment_thread_id_idx": { + "name": "comment_thread_id_idx", + "columns": [ + "threadId" + ], + "isUnique": false + } + }, + "foreignKeys": { + "comment_threadId_comment_thread_id_fk": { + "name": "comment_threadId_comment_thread_id_fk", + "tableFrom": "comment", + "tableTo": "comment_thread", + "columnsFrom": [ + "threadId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "comment_thread": { + "name": "comment_thread", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "scopeKey": { + "name": "scopeKey", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filePath": { + "name": "filePath", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "side": { + "name": "side", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "startLine": { + "name": "startLine", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "endLine": { + "name": "endLine", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "resolvedAt": { + "name": "resolvedAt", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "comment_thread_scope_key_idx": { + "name": "comment_thread_scope_key_idx", + "columns": [ + "scopeKey" + ], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "file_view": { + "name": "file_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "runId": { + "name": "runId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filePath": { + "name": "filePath", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "file_view_user_run_path_unique": { + "name": "file_view_user_run_path_unique", + "columns": [ + "userId", + "runId", + "filePath" + ], + "isUnique": true + } + }, + "foreignKeys": { + "file_view_runId_chapter_run_id_fk": { + "name": "file_view_runId_chapter_run_id_fk", + "tableFrom": "file_view", + "tableTo": "chapter_run", + "columnsFrom": [ + "runId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "key_change": { + "name": "key_change", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "chapterId": { + "name": "chapterId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "externalId": { + "name": "externalId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "content": { + "name": "content", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "lineRefs": { + "name": "lineRefs", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + } + }, + "indexes": { + "key_change_chapter_id_idx": { + "name": "key_change_chapter_id_idx", + "columns": [ + "chapterId" + ], + "isUnique": false + } + }, + "foreignKeys": { + "key_change_chapterId_chapter_id_fk": { + "name": "key_change_chapterId_chapter_id_fk", + "tableFrom": "key_change", + "tableTo": "chapter", + "columnsFrom": [ + "chapterId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "key_change_view": { + "name": "key_change_view", + "columns": { + "id": { + "name": "id", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "createdAt": { + "name": "createdAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updatedAt": { + "name": "updatedAt", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "userId": { + "name": "userId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'local'" + }, + "keyChangeId": { + "name": "keyChangeId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "key_change_view_key_change_id_idx": { + "name": "key_change_view_key_change_id_idx", + "columns": [ + "keyChangeId" + ], + "isUnique": false + }, + "key_change_view_user_key_change_unique": { + "name": "key_change_view_user_key_change_unique", + "columns": [ + "userId", + "keyChangeId" + ], + "isUnique": true + } + }, + "foreignKeys": { + "key_change_view_keyChangeId_key_change_id_fk": { + "name": "key_change_view_keyChangeId_key_change_id_fk", + "tableFrom": "key_change_view", + "tableTo": "key_change", + "columnsFrom": [ + "keyChangeId" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} \ No newline at end of file diff --git a/packages/cli/drizzle/meta/_journal.json b/packages/cli/drizzle/meta/_journal.json index b399d4d..1ea682b 100644 --- a/packages/cli/drizzle/meta/_journal.json +++ b/packages/cli/drizzle/meta/_journal.json @@ -43,6 +43,13 @@ "when": 1780375006767, "tag": "0005_quiet_mimic", "breakpoints": true + }, + { + "idx": 6, + "version": "6", + "when": 1780873869130, + "tag": "0006_puzzling_beast", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/cli/src/db/schema/comment-thread.ts b/packages/cli/src/db/schema/comment-thread.ts new file mode 100644 index 0000000..bce7d27 --- /dev/null +++ b/packages/cli/src/db/schema/comment-thread.ts @@ -0,0 +1,23 @@ +import { index, integer, sqliteTable, text } from "drizzle-orm/sqlite-core"; +import { DIFF_SIDE } from "../../schema.js"; +import { baseColumns } from "./columns.js"; + +export const commentThread = sqliteTable( + "comment_thread", + { + ...baseColumns(), + // Anchors the thread to a diff scope rather than a single run, so comments + // survive re-imports of the same diff (mirrors how external_id keys view-state). + scopeKey: text().notNull(), + filePath: text().notNull(), + side: text({ enum: [DIFF_SIDE.ADDITIONS, DIFF_SIDE.DELETIONS] }).notNull(), + startLine: integer().notNull(), + endLine: integer().notNull(), + /** Null while open; set to the resolution time once resolved. */ + resolvedAt: integer({ mode: "timestamp_ms" }), + }, + (table) => [index("comment_thread_scope_key_idx").on(table.scopeKey)], +); + +export type CommentThreadRow = typeof commentThread.$inferSelect; +export type CommentThreadInsert = typeof commentThread.$inferInsert; diff --git a/packages/cli/src/db/schema/comment.ts b/packages/cli/src/db/schema/comment.ts new file mode 100644 index 0000000..1a9783c --- /dev/null +++ b/packages/cli/src/db/schema/comment.ts @@ -0,0 +1,20 @@ +import { index, sqliteTable, text } from "drizzle-orm/sqlite-core"; +import { LOCAL_USER_ID } from "../local-user.js"; +import { baseColumns } from "./columns.js"; +import { commentThread } from "./comment-thread.js"; + +export const comment = sqliteTable( + "comment", + { + ...baseColumns(), + threadId: text() + .notNull() + .references(() => commentThread.id, { onDelete: "cascade" }), + authorId: text().notNull().default(LOCAL_USER_ID), + body: text().notNull(), + }, + (table) => [index("comment_thread_id_idx").on(table.threadId)], +); + +export type CommentRow = typeof comment.$inferSelect; +export type CommentInsert = typeof comment.$inferInsert; diff --git a/packages/cli/src/db/schema/index.ts b/packages/cli/src/db/schema/index.ts index 8872e14..4677dd2 100644 --- a/packages/cli/src/db/schema/index.ts +++ b/packages/cli/src/db/schema/index.ts @@ -2,6 +2,8 @@ export * from "./chapter.js"; export * from "./chapter-file-view.js"; export * from "./chapter-run.js"; export * from "./chapter-view.js"; +export * from "./comment.js"; +export * from "./comment-thread.js"; export * from "./file-view.js"; export * from "./key-change.js"; export * from "./key-change-view.js"; diff --git a/packages/cli/src/runs/import-chapters.ts b/packages/cli/src/runs/import-chapters.ts index 8530ded..a16119e 100644 --- a/packages/cli/src/runs/import-chapters.ts +++ b/packages/cli/src/runs/import-chapters.ts @@ -4,7 +4,8 @@ import path from "node:path"; import { getDb, type StageDb } from "../db/client.js"; import { chapter, chapterRun, keyChange } from "../db/schema/index.js"; import { type RepoContext, readRepoContext } from "../git.js"; -import { type ChaptersFile, ChaptersFileSchema, SCOPE_KIND, type Scope } from "../schema.js"; +import { type ChaptersFile, ChaptersFileSchema, SCOPE_KIND } from "../schema.js"; +import { deriveScopeKey } from "./scope-key.js"; export interface ImportChaptersResult { runId: string; @@ -27,26 +28,25 @@ export function insertChaptersFile( prNumber: number | null = null, ): ImportChaptersResult { return db.transaction((tx) => { - const [runRow] = tx - .insert(chapterRun) - .values({ - repoRoot: repo.root, - originUrl: repo.originUrl, - prNumber, - scopeKind: file.scope.kind, - workingTreeRef: file.scope.kind === SCOPE_KIND.WORKING_TREE ? file.scope.ref : null, - baseSha: file.scope.baseSha, - headSha: file.scope.headSha, - mergeBaseSha: file.scope.mergeBaseSha, - generatedAt: new Date(file.generatedAt), - prologue: file.prologue ?? null, - }) - .returning({ id: chapterRun.id }) - .all(); + const runValues = { + repoRoot: repo.root, + originUrl: repo.originUrl, + prNumber, + scopeKind: file.scope.kind, + workingTreeRef: file.scope.kind === SCOPE_KIND.WORKING_TREE ? file.scope.ref : null, + baseSha: file.scope.baseSha, + headSha: file.scope.headSha, + mergeBaseSha: file.scope.mergeBaseSha, + generatedAt: new Date(file.generatedAt), + prologue: file.prologue ?? null, + }; + const [runRow] = tx.insert(chapterRun).values(runValues).returning({ id: chapterRun.id }).all(); if (!runRow) throw new Error("chapter_run insert returned no row"); const runId = runRow.id; - const scopeKey = deriveScopeKey(file.scope); + // Shares the run's flattened scope fields so the key matches what the + // comment routes derive from a chapter_run row. + const scopeKey = deriveScopeKey(runValues); let keyChangeCount = 0; for (const c of file.chapters) { @@ -83,13 +83,6 @@ export function insertChaptersFile( }); } -function deriveScopeKey(scope: Scope): string { - if (scope.kind === SCOPE_KIND.COMMITTED) { - return `committed:${scope.baseSha}:${scope.headSha}:${scope.mergeBaseSha}`; - } - return `workingTree:${scope.ref}:${scope.baseSha}:${scope.headSha}:${scope.mergeBaseSha}`; -} - function deriveChapterExternalId(scopeKey: string, agentId: string): string { const hash = createHash("sha256"); hash.update(scopeKey); diff --git a/packages/cli/src/runs/scope-key.ts b/packages/cli/src/runs/scope-key.ts new file mode 100644 index 0000000..494ad89 --- /dev/null +++ b/packages/cli/src/runs/scope-key.ts @@ -0,0 +1,24 @@ +import { SCOPE_KIND, type ScopeKind, type WorkingTreeRef } from "../schema.js"; + +export interface ScopeKeyParts { + scopeKind: ScopeKind; + /** Required for working-tree scopes; null for committed scopes. */ + workingTreeRef: WorkingTreeRef | null; + baseSha: string; + headSha: string; + mergeBaseSha: string; +} + +/** + * Deterministic key identifying a diff scope, independent of any single run. + * Re-imports of the same diff produce the same scope key, which is how review + * state and line-anchored comments survive content regeneration. The shape + * matches a `chapter_run` row so callers can pass one directly. + */ +export function deriveScopeKey(parts: ScopeKeyParts): string { + const { scopeKind, workingTreeRef, baseSha, headSha, mergeBaseSha } = parts; + if (scopeKind === SCOPE_KIND.COMMITTED) { + return `committed:${baseSha}:${headSha}:${mergeBaseSha}`; + } + return `workingTree:${workingTreeRef}:${baseSha}:${headSha}:${mergeBaseSha}`; +} From 3ac9e49f5fb281552e72c3d2f9efdef42dc45bae Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:16:39 -0700 Subject: [PATCH 03/30] feat(PRO-546): add comment thread routes with scope-keyed persistence --- .../cli/src/__tests__/comments.routes.test.ts | 285 ++++++++++++++++ packages/cli/src/routes/comments.ts | 315 ++++++++++++++++++ packages/cli/src/show.ts | 2 + 3 files changed, 602 insertions(+) create mode 100644 packages/cli/src/__tests__/comments.routes.test.ts create mode 100644 packages/cli/src/routes/comments.ts diff --git a/packages/cli/src/__tests__/comments.routes.test.ts b/packages/cli/src/__tests__/comments.routes.test.ts new file mode 100644 index 0000000..35e36ee --- /dev/null +++ b/packages/cli/src/__tests__/comments.routes.test.ts @@ -0,0 +1,285 @@ +import fs from "node:fs/promises"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; +import type { CommentThread, CreateCommentThreadBody } from "@stagereview/types/comments"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { closeDb, getDb } from "../db/client.js"; +import { comment, commentThread } from "../db/schema/index.js"; +import { commentRoutes } from "../routes/comments.js"; +import { insertChaptersFile } from "../runs/import-chapters.js"; +import type { ChaptersFile } from "../schema.js"; +import { LOOPBACK_HOST, type ServerHandle, startServer } from "../server.js"; +import { makeFixture, makeRepoContext } from "./fixtures.js"; + +let tmpDir: string; +let dbPath: string; +let webDist: string; +const handles: ServerHandle[] = []; + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-cli-comments-")); + dbPath = path.join(tmpDir, "db.sqlite"); + webDist = path.join(tmpDir, "web-dist"); + await fs.mkdir(webDist); + await fs.writeFile(path.join(webDist, "index.html"), ""); + closeDb(); +}); + +afterEach(async () => { + while (handles.length > 0) { + const h = handles.pop(); + if (h) await h.close(); + } + closeDb(); + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +async function startWithRoutes(): Promise { + const db = getDb({ dbPath }); + const handle = await startServer({ webDistPath: webDist, routes: commentRoutes(db) }); + handles.push(handle); + return handle; +} + +interface JsonResponse { + status: number; + body: unknown; +} + +function send( + port: number, + method: string, + requestPath: string, + body?: unknown, +): Promise { + const payload = body === undefined ? "" : JSON.stringify(body); + return new Promise((resolve, reject) => { + const req = http.request( + { + hostname: LOOPBACK_HOST, + port, + method, + path: requestPath, + agent: false, + headers: { + "Content-Type": "application/json", + "Content-Length": Buffer.byteLength(payload).toString(), + }, + }, + (res) => { + const chunks: Buffer[] = []; + res.on("data", (c: Buffer) => chunks.push(c)); + res.on("end", () => { + const text = Buffer.concat(chunks).toString("utf8"); + resolve({ status: res.statusCode ?? 0, body: text ? JSON.parse(text) : null }); + }); + }, + ); + req.on("error", reject); + if (payload) req.write(payload); + req.end(); + }); +} + +function seedRun(over: Partial = {}): string { + const db = getDb({ dbPath }); + return insertChaptersFile(db, makeFixture(over), makeRepoContext()).runId; +} + +function makeThreadBody(over: Partial = {}): CreateCommentThreadBody { + return { + filePath: "src/foo.ts", + side: "additions", + startLine: 5, + endLine: 10, + body: "Why does this fall back to the primary org?", + ...over, + }; +} + +async function createThread( + port: number, + runId: string, + over: Partial = {}, +): Promise { + const res = await send(port, "POST", `/api/runs/${runId}/comment-threads`, makeThreadBody(over)); + expect(res.status).toBe(201); + return res.body as CommentThread; +} + +describe("comment threads API", () => { + it("POST creates a thread with its root comment and the anchor", async () => { + const runId = seedRun(); + const { port } = await startWithRoutes(); + + const thread = await createThread(port, runId, { body: "First!" }); + expect(thread.filePath).toBe("src/foo.ts"); + expect(thread.side).toBe("additions"); + expect(thread.startLine).toBe(5); + expect(thread.endLine).toBe(10); + expect(thread.resolvedAt).toBeNull(); + expect(thread.comments).toHaveLength(1); + expect(thread.comments[0]?.body).toBe("First!"); + expect(thread.comments[0]?.authorId).toBe("local"); + + const db = getDb({ dbPath }); + expect(db.select().from(commentThread).all()).toHaveLength(1); + expect(db.select().from(comment).all()).toHaveLength(1); + }); + + it("GET lists threads (oldest comment first) and returns [] when empty", async () => { + const runId = seedRun(); + const { port } = await startWithRoutes(); + + const empty = await send(port, "GET", `/api/runs/${runId}/comment-threads`); + expect(empty.body).toEqual([]); + + const thread = await createThread(port, runId); + await send(port, "POST", `/api/comment-threads/${thread.id}/replies`, { body: "A reply" }); + + const list = await send(port, "GET", `/api/runs/${runId}/comment-threads`); + const threads = list.body as CommentThread[]; + expect(threads).toHaveLength(1); + expect(threads[0]?.comments.map((c) => c.body)).toEqual([ + "Why does this fall back to the primary org?", + "A reply", + ]); + }); + + it("PATCH toggles a thread's resolved state", async () => { + const runId = seedRun(); + const { port } = await startWithRoutes(); + const thread = await createThread(port, runId); + + const resolved = await send(port, "PATCH", `/api/comment-threads/${thread.id}`, { + resolved: true, + }); + expect((resolved.body as CommentThread).resolvedAt).not.toBeNull(); + + const reopened = await send(port, "PATCH", `/api/comment-threads/${thread.id}`, { + resolved: false, + }); + expect((reopened.body as CommentThread).resolvedAt).toBeNull(); + }); + + it("PATCH edits a comment body", async () => { + const runId = seedRun(); + const { port } = await startWithRoutes(); + const thread = await createThread(port, runId); + const commentId = thread.comments[0]?.id; + + const res = await send(port, "PATCH", `/api/comments/${commentId}`, { body: "Edited" }); + expect(res.status).toBe(200); + expect((res.body as { body: string }).body).toBe("Edited"); + }); + + it("DELETE comment keeps the thread while other comments remain, removes it when last", async () => { + const runId = seedRun(); + const { port } = await startWithRoutes(); + const thread = await createThread(port, runId); + const reply = await send(port, "POST", `/api/comment-threads/${thread.id}/replies`, { + body: "Reply", + }); + const replyId = (reply.body as { id: string }).id; + const rootId = thread.comments[0]?.id; + + await send(port, "DELETE", `/api/comments/${replyId}`); + const afterReplyDelete = await send(port, "GET", `/api/runs/${runId}/comment-threads`); + expect(afterReplyDelete.body as CommentThread[]).toHaveLength(1); + + await send(port, "DELETE", `/api/comments/${rootId}`); + const afterRootDelete = await send(port, "GET", `/api/runs/${runId}/comment-threads`); + expect(afterRootDelete.body as CommentThread[]).toHaveLength(0); + }); + + it("DELETE thread cascades to its comments and is idempotent", async () => { + const runId = seedRun(); + const { port } = await startWithRoutes(); + const thread = await createThread(port, runId); + await send(port, "POST", `/api/comment-threads/${thread.id}/replies`, { body: "Reply" }); + + const first = await send(port, "DELETE", `/api/comment-threads/${thread.id}`); + expect(first.status).toBe(200); + const db = getDb({ dbPath }); + expect(db.select().from(commentThread).all()).toHaveLength(0); + expect(db.select().from(comment).all()).toHaveLength(0); + + const second = await send(port, "DELETE", `/api/comment-threads/${thread.id}`); + expect(second.status).toBe(200); + }); + + it("threads survive re-import of the same diff scope", async () => { + // Two imports with identical scope create two runs sharing one scope key. + // A thread created against the first run must be visible from the second. + const runA = seedRun(); + const { port } = await startWithRoutes(); + await createThread(port, runA, { body: "Survives regeneration" }); + + const runB = seedRun(); + expect(runB).not.toBe(runA); + + const viaB = await send(port, "GET", `/api/runs/${runB}/comment-threads`); + const threads = viaB.body as CommentThread[]; + expect(threads).toHaveLength(1); + expect(threads[0]?.comments[0]?.body).toBe("Survives regeneration"); + }); + + it("threads are isolated across different diff scopes", async () => { + const runA = seedRun({ + scope: { + kind: "committed", + baseSha: "a".repeat(40), + headSha: "b".repeat(40), + mergeBaseSha: "c".repeat(40), + }, + }); + const runB = seedRun({ + scope: { + kind: "committed", + baseSha: "d".repeat(40), + headSha: "e".repeat(40), + mergeBaseSha: "f".repeat(40), + }, + }); + const { port } = await startWithRoutes(); + await createThread(port, runA); + + const viaB = await send(port, "GET", `/api/runs/${runB}/comment-threads`); + expect(viaB.body as CommentThread[]).toHaveLength(0); + }); + + it("returns 404 for an unknown run on GET and POST", async () => { + const unknown = "00000000-0000-0000-0000-000000000000"; + const { port } = await startWithRoutes(); + + const get = await send(port, "GET", `/api/runs/${unknown}/comment-threads`); + expect(get.status).toBe(404); + const post = await send(port, "POST", `/api/runs/${unknown}/comment-threads`, makeThreadBody()); + expect(post.status).toBe(404); + }); + + it("returns 404 when replying to an unknown thread", async () => { + const { port } = await startWithRoutes(); + const res = await send(port, "POST", "/api/comment-threads/nope/replies", { body: "hi" }); + expect(res.status).toBe(404); + }); + + it("returns 400 for an empty body or an inverted line range", async () => { + const runId = seedRun(); + const { port } = await startWithRoutes(); + + const emptyBody = await send(port, "POST", `/api/runs/${runId}/comment-threads`, { + ...makeThreadBody(), + body: "", + }); + expect(emptyBody.status).toBe(400); + + const inverted = await send(port, "POST", `/api/runs/${runId}/comment-threads`, { + ...makeThreadBody(), + startLine: 10, + endLine: 5, + }); + expect(inverted.status).toBe(400); + }); +}); diff --git a/packages/cli/src/routes/comments.ts b/packages/cli/src/routes/comments.ts new file mode 100644 index 0000000..b651943 --- /dev/null +++ b/packages/cli/src/routes/comments.ts @@ -0,0 +1,315 @@ +import { + CommentBodySchema, + type Comment as CommentDto, + type CommentThread as CommentThreadDto, + CreateCommentThreadBodySchema, + ResolveThreadBodySchema, +} from "@stagereview/types/comments"; +import { asc, eq, inArray } from "drizzle-orm"; +import type { z } from "zod"; +import type { StageDb } from "../db/client.js"; +import { LOCAL_USER_ID } from "../db/local-user.js"; +import { + type CommentRow, + type CommentThreadRow, + chapterRun, + comment, + commentThread, +} from "../db/schema/index.js"; +import { deriveScopeKey } from "../runs/scope-key.js"; +import type { Route } from "../server.js"; +import { readJsonBody, writeJson } from "./json.js"; + +export function commentRoutes(db: StageDb): Route[] { + return [ + // Threads are anchored to a diff scope, not a run, so they survive re-imports + // of the same diff. We resolve the run's scope key and key every query off it. + { + method: "GET", + pattern: "/api/runs/:runId/comment-threads", + handler: (_req, res, params) => { + const scopeKey = resolveRunScopeKey(db, params.runId); + if (scopeKey === null) { + writeJson(res, 404, { error: `Run ${params.runId} not found` }); + return; + } + writeJson(res, 200, listThreads(db, scopeKey)); + }, + }, + { + method: "POST", + pattern: "/api/runs/:runId/comment-threads", + handler: async (req, res, params) => { + const scopeKey = resolveRunScopeKey(db, params.runId); + if (scopeKey === null) { + writeJson(res, 404, { error: `Run ${params.runId} not found` }); + return; + } + const body = await parseBody(req, res, CreateCommentThreadBodySchema); + if (!body) return; + + const created = db.transaction((tx) => { + const [threadRow] = tx + .insert(commentThread) + .values({ + scopeKey, + filePath: body.filePath, + side: body.side, + startLine: body.startLine, + endLine: body.endLine, + }) + .returning() + .all(); + if (!threadRow) throw new Error("comment_thread insert returned no row"); + const [commentRow] = tx + .insert(comment) + .values({ threadId: threadRow.id, authorId: LOCAL_USER_ID, body: body.body }) + .returning() + .all(); + if (!commentRow) throw new Error("comment insert returned no row"); + return toThreadDto(threadRow, [commentRow]); + }); + writeJson(res, 201, created); + }, + }, + { + method: "POST", + pattern: "/api/comment-threads/:threadId/replies", + handler: async (req, res, params) => { + const threadId = params.threadId; + if (!threadId || !threadExists(db, threadId)) { + writeJson(res, 404, { error: `Thread ${params.threadId} not found` }); + return; + } + const body = await parseBody(req, res, CommentBodySchema); + if (!body) return; + + const created = db.transaction((tx) => { + const [commentRow] = tx + .insert(comment) + .values({ threadId, authorId: LOCAL_USER_ID, body: body.body }) + .returning() + .all(); + if (!commentRow) throw new Error("comment insert returned no row"); + // Bump the thread so its updatedAt reflects the latest activity. + tx.update(commentThread) + .set({ updatedAt: new Date() }) + .where(eq(commentThread.id, threadId)) + .run(); + return toCommentDto(commentRow); + }); + writeJson(res, 201, created); + }, + }, + { + method: "PATCH", + pattern: "/api/comment-threads/:threadId", + handler: async (req, res, params) => { + const threadId = params.threadId; + if (!threadId) { + writeJson(res, 400, { error: "Missing threadId" }); + return; + } + const body = await parseBody(req, res, ResolveThreadBodySchema); + if (!body) return; + + const [updated] = db + .update(commentThread) + .set({ resolvedAt: body.resolved ? new Date() : null }) + .where(eq(commentThread.id, threadId)) + .returning() + .all(); + if (!updated) { + writeJson(res, 404, { error: `Thread ${threadId} not found` }); + return; + } + writeJson(res, 200, toThreadDto(updated, threadComments(db, threadId))); + }, + }, + { + method: "DELETE", + pattern: "/api/comment-threads/:threadId", + handler: (_req, res, params) => { + const threadId = params.threadId; + if (!threadId) { + writeJson(res, 400, { error: "Missing threadId" }); + return; + } + // Idempotent: deleting an absent thread is a no-op. The cascade FK + // removes the thread's comments. + db.delete(commentThread).where(eq(commentThread.id, threadId)).run(); + writeJson(res, 200, {}); + }, + }, + { + method: "PATCH", + pattern: "/api/comments/:commentId", + handler: async (req, res, params) => { + const commentId = params.commentId; + if (!commentId) { + writeJson(res, 400, { error: "Missing commentId" }); + return; + } + const body = await parseBody(req, res, CommentBodySchema); + if (!body) return; + + const [updated] = db + .update(comment) + .set({ body: body.body }) + .where(eq(comment.id, commentId)) + .returning() + .all(); + if (!updated) { + writeJson(res, 404, { error: `Comment ${commentId} not found` }); + return; + } + writeJson(res, 200, toCommentDto(updated)); + }, + }, + { + method: "DELETE", + pattern: "/api/comments/:commentId", + handler: (_req, res, params) => { + const commentId = params.commentId; + if (!commentId) { + writeJson(res, 400, { error: "Missing commentId" }); + return; + } + // Deleting the last comment removes its now-empty thread so no + // dangling anchors linger. Idempotent for an absent comment. + db.transaction((tx) => { + const [row] = tx + .select({ threadId: comment.threadId }) + .from(comment) + .where(eq(comment.id, commentId)) + .limit(1) + .all(); + if (!row) return; + tx.delete(comment).where(eq(comment.id, commentId)).run(); + const remaining = tx + .select({ id: comment.id }) + .from(comment) + .where(eq(comment.threadId, row.threadId)) + .limit(1) + .all(); + if (remaining.length === 0) { + tx.delete(commentThread).where(eq(commentThread.id, row.threadId)).run(); + } + }); + writeJson(res, 200, {}); + }, + }, + ]; +} + +function resolveRunScopeKey(db: StageDb, runId: string | undefined): string | null { + if (!runId) return null; + const [run] = db + .select({ + scopeKind: chapterRun.scopeKind, + workingTreeRef: chapterRun.workingTreeRef, + baseSha: chapterRun.baseSha, + headSha: chapterRun.headSha, + mergeBaseSha: chapterRun.mergeBaseSha, + }) + .from(chapterRun) + .where(eq(chapterRun.id, runId)) + .limit(1) + .all(); + if (!run) return null; + return deriveScopeKey(run); +} + +function listThreads(db: StageDb, scopeKey: string): CommentThreadDto[] { + const threads = db + .select() + .from(commentThread) + .where(eq(commentThread.scopeKey, scopeKey)) + .orderBy(asc(commentThread.createdAt)) + .all(); + if (threads.length === 0) return []; + + const comments = db + .select() + .from(comment) + .where( + inArray( + comment.threadId, + threads.map((t) => t.id), + ), + ) + .orderBy(asc(comment.createdAt)) + .all(); + + const byThread = new Map(); + for (const c of comments) { + const list = byThread.get(c.threadId); + if (list) list.push(c); + else byThread.set(c.threadId, [c]); + } + return threads.map((t) => toThreadDto(t, byThread.get(t.id) ?? [])); +} + +function threadComments(db: StageDb, threadId: string): CommentRow[] { + return db + .select() + .from(comment) + .where(eq(comment.threadId, threadId)) + .orderBy(asc(comment.createdAt)) + .all(); +} + +function threadExists(db: StageDb, threadId: string): boolean { + return ( + db + .select({ id: commentThread.id }) + .from(commentThread) + .where(eq(commentThread.id, threadId)) + .limit(1) + .all().length > 0 + ); +} + +function toThreadDto(thread: CommentThreadRow, comments: CommentRow[]): CommentThreadDto { + return { + id: thread.id, + filePath: thread.filePath, + side: thread.side, + startLine: thread.startLine, + endLine: thread.endLine, + resolvedAt: thread.resolvedAt?.toISOString() ?? null, + createdAt: thread.createdAt.toISOString(), + updatedAt: thread.updatedAt.toISOString(), + comments: comments.map(toCommentDto), + }; +} + +function toCommentDto(row: CommentRow): CommentDto { + return { + id: row.id, + body: row.body, + authorId: row.authorId, + createdAt: row.createdAt.toISOString(), + updatedAt: row.updatedAt.toISOString(), + }; +} + +async function parseBody( + req: Parameters[0], + res: Parameters[1], + schema: z.ZodType, +): Promise { + let raw: unknown; + try { + raw = await readJsonBody(req); + } catch (err) { + writeJson(res, 400, { error: err instanceof Error ? err.message : "Invalid JSON body" }); + return null; + } + const parsed = schema.safeParse(raw); + if (!parsed.success) { + writeJson(res, 400, { error: parsed.error.issues[0]?.message ?? "Invalid request body" }); + return null; + } + return parsed.data; +} diff --git a/packages/cli/src/show.ts b/packages/cli/src/show.ts index 04b8e4e..40e1ef8 100644 --- a/packages/cli/src/show.ts +++ b/packages/cli/src/show.ts @@ -6,6 +6,7 @@ import { closeDb, getDb } from "./db/client.js"; import { parseGitDiff } from "./diff-parser.js"; import { filterFilesForLlm, loadStageIgnore } from "./filter-files.js"; import { readRepoContext, readRepoRoot } from "./git.js"; +import { commentRoutes } from "./routes/comments.js"; import { diffRoutes } from "./routes/diff.js"; import { pullRequestRoutes } from "./routes/pull-request.js"; import { pullRequestMutationRoutes } from "./routes/pull-request-mutations.js"; @@ -33,6 +34,7 @@ export async function show(jsonPath: string, options: DiffScopeOptions): Promise routes: [ ...runRoutes(db), ...viewStateRoutes(db), + ...commentRoutes(db), ...diffRoutes(db), ...pullRequestRoutes(db), ...pullRequestMutationRoutes(db), From fcb8710932ae413d4d0bd414ec1cd43e26913909 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:20:34 -0700 Subject: [PATCH 04/30] feat(PRO-546): add comment threads data hook, context, and text-selection --- packages/web/src/app/runs.$runId.tsx | 5 +- .../web/src/lib/comment-threads-context.tsx | 27 ++ packages/web/src/lib/use-comment-threads.ts | 159 +++++++++++ packages/web/src/lib/use-text-selection.ts | 262 ++++++++++++++++++ 4 files changed, 451 insertions(+), 2 deletions(-) create mode 100644 packages/web/src/lib/comment-threads-context.tsx create mode 100644 packages/web/src/lib/use-comment-threads.ts create mode 100644 packages/web/src/lib/use-text-selection.ts diff --git a/packages/web/src/app/runs.$runId.tsx b/packages/web/src/app/runs.$runId.tsx index 4e3a8e1..0ed666b 100644 --- a/packages/web/src/app/runs.$runId.tsx +++ b/packages/web/src/app/runs.$runId.tsx @@ -1,5 +1,6 @@ import { createFileRoute } from "@tanstack/react-router"; import { Topbar } from "@/components/layout/topbar"; +import { CommentThreadsProvider } from "@/lib/comment-threads-context"; import { PullRequestLayout } from "@/routes/pull-request-layout"; export const Route = createFileRoute("/runs/$runId")({ @@ -9,9 +10,9 @@ export const Route = createFileRoute("/runs/$runId")({ function RunLayout() { const { runId } = Route.useParams(); return ( - <> + - + ); } diff --git a/packages/web/src/lib/comment-threads-context.tsx b/packages/web/src/lib/comment-threads-context.tsx new file mode 100644 index 0000000..8100b24 --- /dev/null +++ b/packages/web/src/lib/comment-threads-context.tsx @@ -0,0 +1,27 @@ +import { createContext, type ReactNode, useContext } from "react"; +import { type UseCommentThreadsResult, useCommentThreads } from "./use-comment-threads"; + +const CommentThreadsContext = createContext(null); + +/** + * Provides the run's comment threads + mutations to the diff tree without + * prop-drilling through FileDiffList. Mounted once at the run layout. + */ +export function CommentThreadsProvider({ + runId, + children, +}: { + runId: string; + children: ReactNode; +}) { + const value = useCommentThreads(runId); + return {children}; +} + +export function useCommentThreadsContext(): UseCommentThreadsResult { + const ctx = useContext(CommentThreadsContext); + if (!ctx) { + throw new Error("useCommentThreadsContext must be used within a CommentThreadsProvider"); + } + return ctx; +} diff --git a/packages/web/src/lib/use-comment-threads.ts b/packages/web/src/lib/use-comment-threads.ts new file mode 100644 index 0000000..972e3f9 --- /dev/null +++ b/packages/web/src/lib/use-comment-threads.ts @@ -0,0 +1,159 @@ +import { + type Comment, + type CommentThread, + CommentThreadsResponseSchema, + type CreateCommentThreadBody, +} from "@stagereview/types/comments"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { useMemo } from "react"; +import { jsonFetch } from "./use-view-state"; + +export type { Comment, CommentThread, CreateCommentThreadBody }; + +const COMMENT_THREADS_ROOT = "comment-threads"; + +export function commentThreadsQueryKey(runId: string): readonly unknown[] { + return [COMMENT_THREADS_ROOT, runId]; +} + +async function fetchCommentThreads(runId: string): Promise { + // Parse at the boundary so server-side schema drift surfaces as a query error + // here, not as a render crash deeper in the diff. + const raw = await jsonFetch(`/api/runs/${encodeURIComponent(runId)}/comment-threads`); + return CommentThreadsResponseSchema.parse(raw); +} + +const jsonRequest = (method: string, body?: unknown): RequestInit => ({ + method, + headers: { "Content-Type": "application/json" }, + body: body === undefined ? undefined : JSON.stringify(body), +}); + +export interface UseCommentThreadsResult { + threads: CommentThread[]; + /** Stable reference; rebuilt only when the underlying query data changes. */ + threadsByFile: ReadonlyMap; + isLoading: boolean; + error: unknown; + createThread: (input: CreateCommentThreadBody) => Promise; + replyToThread: (input: { threadId: string; body: string }) => Promise; + setThreadResolved: (input: { threadId: string; resolved: boolean }) => Promise; + editComment: (input: { commentId: string; body: string }) => Promise; + deleteThread: (threadId: string) => Promise; + deleteComment: (commentId: string) => Promise; +} + +/** + * Loads the comment threads anchored to a run's diff scope and exposes the + * thread/comment mutations. The local server commits synchronously, so each + * mutation simply invalidates the query rather than maintaining optimistic + * caches — the refetch round-trip is effectively instant. + */ +export function useCommentThreads(runId: string): UseCommentThreadsResult { + const queryClient = useQueryClient(); + const queryKey = useMemo(() => commentThreadsQueryKey(runId), [runId]); + + const { data, isLoading, error } = useQuery({ + queryKey, + queryFn: () => fetchCommentThreads(runId), + enabled: runId !== "", + }); + + const threads = useMemo(() => data ?? [], [data]); + const threadsByFile = useMemo(() => groupByFile(threads), [threads]); + + const invalidate = () => queryClient.invalidateQueries({ queryKey }); + + const createMutation = useMutation({ + mutationFn: (input: CreateCommentThreadBody) => + jsonFetch( + `/api/runs/${encodeURIComponent(runId)}/comment-threads`, + jsonRequest("POST", input), + ), + onSuccess: invalidate, + }); + + const replyMutation = useMutation({ + mutationFn: async ({ threadId, body }: { threadId: string; body: string }) => { + await jsonFetch( + `/api/comment-threads/${encodeURIComponent(threadId)}/replies`, + jsonRequest("POST", { body }), + ); + }, + onSuccess: invalidate, + }); + + const resolveMutation = useMutation({ + mutationFn: async ({ threadId, resolved }: { threadId: string; resolved: boolean }) => { + await jsonFetch( + `/api/comment-threads/${encodeURIComponent(threadId)}`, + jsonRequest("PATCH", { resolved }), + ); + }, + onSuccess: invalidate, + }); + + const editMutation = useMutation({ + mutationFn: async ({ commentId, body }: { commentId: string; body: string }) => { + await jsonFetch( + `/api/comments/${encodeURIComponent(commentId)}`, + jsonRequest("PATCH", { body }), + ); + }, + onSuccess: invalidate, + }); + + const deleteThreadMutation = useMutation({ + mutationFn: async (threadId: string) => { + await jsonFetch( + `/api/comment-threads/${encodeURIComponent(threadId)}`, + jsonRequest("DELETE"), + ); + }, + onSuccess: invalidate, + }); + + const deleteCommentMutation = useMutation({ + mutationFn: async (commentId: string) => { + await jsonFetch(`/api/comments/${encodeURIComponent(commentId)}`, jsonRequest("DELETE")); + }, + onSuccess: invalidate, + }); + + return useMemo( + () => ({ + threads, + threadsByFile, + isLoading, + error, + createThread: createMutation.mutateAsync, + replyToThread: replyMutation.mutateAsync, + setThreadResolved: resolveMutation.mutateAsync, + editComment: editMutation.mutateAsync, + deleteThread: deleteThreadMutation.mutateAsync, + deleteComment: deleteCommentMutation.mutateAsync, + }), + [ + threads, + threadsByFile, + isLoading, + error, + createMutation.mutateAsync, + replyMutation.mutateAsync, + resolveMutation.mutateAsync, + editMutation.mutateAsync, + deleteThreadMutation.mutateAsync, + deleteCommentMutation.mutateAsync, + ], + ); +} + +function groupByFile(threads: CommentThread[]): ReadonlyMap { + const map = new Map(); + for (const thread of threads) { + const list = map.get(thread.filePath); + if (list) list.push(thread); + else map.set(thread.filePath, [thread]); + } + return map; +} diff --git a/packages/web/src/lib/use-text-selection.ts b/packages/web/src/lib/use-text-selection.ts new file mode 100644 index 0000000..5bb5177 --- /dev/null +++ b/packages/web/src/lib/use-text-selection.ts @@ -0,0 +1,262 @@ +import type { SelectedLineRange } from "@pierre/diffs"; +import { useCallback, useEffect, useRef, useState } from "react"; +import { DIFF_SIDE, type DiffSide } from "@/lib/diff-types"; + +export interface TextSelectionInfo { + /** Bounding rect of the browser selection in page (document) coordinates. */ + rect: DOMRect; + /** Pierre line range derived from the selection. */ + lineRange: SelectedLineRange; +} + +interface SelectedLineRangeParams { + startLine: number; + endLine: number; + startSide: DiffSide; + endSide: DiffSide; +} + +/** + * Builds a Pierre line range from selection endpoints. A comment can't span both + * diff sides in split view, so cross-side selections return null. + */ +export function buildSelectedLineRange({ + startLine, + endLine, + startSide, + endSide, +}: SelectedLineRangeParams): SelectedLineRange | null { + if (startSide !== endSide) return null; + return { + start: Math.min(startLine, endLine), + side: startSide, + end: Math.max(startLine, endLine), + endSide: startSide, + }; +} + +/** + * Normalizes a Pierre line range so `start <= end`. Pierre's drag-to-select emits + * `{ start: anchor, end: currentLine }` without ordering them, so dragging upward + * produces a range where `start > end`. Swap endpoints (and sides) so downstream + * consumers can rely on ascending order. + */ +export function normalizeSelectedLineRange(range: SelectedLineRange): SelectedLineRange { + if (range.start <= range.end) return range; + return { + start: range.end, + side: range.endSide ?? range.side, + end: range.start, + endSide: range.side, + }; +} + +/** + * Finds the closest ancestor element (including the node itself) with a + * `data-line` attribute. Does not cross shadow DOM boundaries. + */ +function findLineElement(node: Node): HTMLElement | null { + let current: Node | null = node; + while (current) { + if (current instanceof HTMLElement && current.hasAttribute("data-line")) { + return current; + } + current = current.parentElement; + } + return null; +} + +/** + * Determines a line element's diff side from its `data-additions`/`data-deletions` + * ancestor, falling back to its `data-line-type` for unified/single-sided diffs. + */ +function getLineSide(lineEl: HTMLElement): DiffSide { + let current: HTMLElement | null = lineEl; + while (current) { + if (current.hasAttribute("data-additions")) return DIFF_SIDE.ADDITIONS; + if (current.hasAttribute("data-deletions")) return DIFF_SIDE.DELETIONS; + current = current.parentElement; + } + const lineType = lineEl.getAttribute("data-line-type"); + if (lineType === "deletion") return DIFF_SIDE.DELETIONS; + return DIFF_SIDE.ADDITIONS; +} + +/** Chrome exposes `getSelection()` on `ShadowRoot` (non-standard); detect it at runtime. */ +function hasGetSelection( + root: ShadowRoot | null, +): root is ShadowRoot & { getSelection: () => Selection | null } { + return root != null && "getSelection" in root; +} + +function getLineNumber(el: HTMLElement): number { + return Number(el.getAttribute("data-line")); +} + +/** + * Detects native text selection inside a Pierre diff container and converts it to + * a {@link TextSelectionInfo} (line range + bounding rect). Returns null when + * there's no active selection in the diff. + */ +export function useTextSelection(containerRef: React.RefObject) { + const [selectionInfo, setSelectionInfo] = useState(null); + const isMouseDownRef = useRef(false); + const rafIdRef = useRef(null); + const shadowRootRef = useRef(null); + + const clearSelection = useCallback(() => { + setSelectionInfo(null); + window.getSelection()?.removeAllRanges(); + if (hasGetSelection(shadowRootRef.current)) { + shadowRootRef.current.getSelection()?.removeAllRanges(); + } + }, []); + + // Dismiss the popup when the user clicks outside this diff container (e.g. + // selecting text in another file's diff). The popup is portaled to body, so + // don't dismiss when the click lands on the popup itself. + useEffect(() => { + function handleGlobalMouseDown(e: MouseEvent) { + const container = containerRef.current; + if (!container) return; + if (!(e.target instanceof Node)) return; + if (container.contains(e.target)) return; + if (e.target instanceof Element && e.target.closest("[data-text-selection-popup]")) return; + setSelectionInfo(null); + } + document.addEventListener("mousedown", handleGlobalMouseDown, true); + return () => document.removeEventListener("mousedown", handleGlobalMouseDown, true); + }, [containerRef]); + + useEffect(() => { + const container = containerRef.current; + if (!container) return; + + // Pierre renders inside a shadow DOM (). Events from inside + // the shadow root don't reach capture listeners on the outer div, so listen + // on the shadow root directly too. + const diffsContainer = container.querySelector("diffs-container"); + const shadowRoot = diffsContainer?.shadowRoot ?? null; + shadowRootRef.current = shadowRoot; + + const handleMouseDown = () => { + isMouseDownRef.current = true; + if (rafIdRef.current !== null) { + cancelAnimationFrame(rafIdRef.current); + rafIdRef.current = null; + } + setSelectionInfo(null); + }; + + const handleMouseUp = () => { + if (!isMouseDownRef.current) return; + isMouseDownRef.current = false; + + // Defer a frame so the browser finalizes the selection first. + if (rafIdRef.current !== null) cancelAnimationFrame(rafIdRef.current); + rafIdRef.current = requestAnimationFrame(() => { + rafIdRef.current = null; + const selection = hasGetSelection(shadowRoot) + ? shadowRoot.getSelection() + : window.getSelection(); + if (!selection || selection.isCollapsed || selection.rangeCount === 0) return; + + const range = selection.getRangeAt(0); + const startLineEl = findLineElement(range.startContainer); + const endLineEl = findLineElement(range.endContainer); + if (!startLineEl || !endLineEl) return; + + // Confirm the selection is within our diff container, walking up through + // any shadow DOM hosts. + let ancestor: Node | null = startLineEl; + let insideContainer = false; + while (ancestor) { + if (ancestor === container) { + insideContainer = true; + break; + } + ancestor = ancestor instanceof ShadowRoot ? ancestor.host : ancestor.parentNode; + } + if (!insideContainer) return; + + const startLine = getLineNumber(startLineEl); + let endLine = getLineNumber(endLineEl); + const startSide = getLineSide(startLineEl); + let endSide = getLineSide(endLineEl); + + // Triple-click selects a full line but the browser extends the range to + // offset 0 of the next line. Pull the end back to the line with content. + const isTripleClick = endLine > startLine && range.endOffset === 0; + if (isTripleClick) { + endLine--; + if (endLine === startLine) { + endSide = startSide; + } else { + const sideContainer = + endLineEl.closest("[data-additions], [data-deletions]") ?? shadowRoot ?? container; + const adjustedEl = sideContainer.querySelector(`[data-line="${endLine}"]`); + if (adjustedEl) endSide = getLineSide(adjustedEl); + } + } + + // getClientRects() yields tight bounds around the selected text. Multi-line + // selections also include full-width block rects (from the line
s); + // filter those so only inline text fragments contribute. For triple-click, + // clamp to the start line so the next line's rects don't skew bounds. + const rectRange = isTripleClick ? range.cloneRange() : range; + if (isTripleClick) rectRange.setEndAfter(startLineEl); + const lineWidth = startLineEl.getBoundingClientRect().width; + const clientRects = rectRange.getClientRects(); + let left = Infinity; + let right = -Infinity; + let top = Infinity; + let bottom = -Infinity; + for (const r of clientRects) { + if (r.width === 0 && r.height === 0) continue; + if (Math.abs(r.width - lineWidth) < 1) continue; + left = Math.min(left, r.left); + right = Math.max(right, r.right); + top = Math.min(top, r.top); + bottom = Math.max(bottom, r.bottom); + } + if (!Number.isFinite(left)) { + const rangeRect = range.getBoundingClientRect(); + left = rangeRect.left; + right = rangeRect.right; + top = rangeRect.top; + bottom = rangeRect.bottom; + } + + const rect = new DOMRect( + left + window.scrollX, + top + window.scrollY, + right - left, + bottom - top, + ); + + const lineRange = buildSelectedLineRange({ startLine, endLine, startSide, endSide }); + if (!lineRange) return; + setSelectionInfo({ rect, lineRange }); + }); + }; + + container.addEventListener("mousedown", handleMouseDown, true); + container.addEventListener("mouseup", handleMouseUp, true); + if (shadowRoot) { + shadowRoot.addEventListener("mousedown", handleMouseDown, true); + shadowRoot.addEventListener("mouseup", handleMouseUp, true); + } + + return () => { + if (rafIdRef.current !== null) cancelAnimationFrame(rafIdRef.current); + container.removeEventListener("mousedown", handleMouseDown, true); + container.removeEventListener("mouseup", handleMouseUp, true); + if (shadowRoot) { + shadowRoot.removeEventListener("mousedown", handleMouseDown, true); + shadowRoot.removeEventListener("mouseup", handleMouseUp, true); + } + }; + }, [containerRef]); + + return { selectionInfo, clearSelection }; +} From 92ad9225ecb7aec34e49ed9998c74e6d0c03f696 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:25:34 -0700 Subject: [PATCH 05/30] feat(PRO-546): vendor comment thread + composer UI components --- packages/web/package.json | 1 + .../components/comments/comment-actions.tsx | 41 +++ .../src/components/comments/comment-form.tsx | 100 ++++++ .../comments/comment-markdown-editor.tsx | 97 ++++++ .../components/comments/comment-thread.tsx | 323 ++++++++++++++++++ 5 files changed, 562 insertions(+) create mode 100644 packages/web/src/components/comments/comment-actions.tsx create mode 100644 packages/web/src/components/comments/comment-form.tsx create mode 100644 packages/web/src/components/comments/comment-markdown-editor.tsx create mode 100644 packages/web/src/components/comments/comment-thread.tsx diff --git a/packages/web/package.json b/packages/web/package.json index b519ccc..3b96d64 100644 --- a/packages/web/package.json +++ b/packages/web/package.json @@ -34,6 +34,7 @@ "react-dom": "^19.2.3", "react-hotkeys-hook": "^5.3.0", "react-markdown": "^10.1.0", + "react-textarea-autosize": "^8.5.9", "react-zoom-pan-pinch": "^3.7.0", "rehype-raw": "^7.0.0", "rehype-sanitize": "^6.0.0", diff --git a/packages/web/src/components/comments/comment-actions.tsx b/packages/web/src/components/comments/comment-actions.tsx new file mode 100644 index 0000000..bd3cb7b --- /dev/null +++ b/packages/web/src/components/comments/comment-actions.tsx @@ -0,0 +1,41 @@ +import { MoreHorizontal, Pencil, Trash2 } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "@/components/ui/dropdown-menu"; + +interface CommentActionsProps { + onEdit: () => void; + onDelete: () => void; + deleteLabel?: string; +} + +export function CommentActions({ onEdit, onDelete, deleteLabel = "Delete" }: CommentActionsProps) { + return ( + + + + + + + + Edit + + + + {deleteLabel} + + + + ); +} diff --git a/packages/web/src/components/comments/comment-form.tsx b/packages/web/src/components/comments/comment-form.tsx new file mode 100644 index 0000000..c970a1c --- /dev/null +++ b/packages/web/src/components/comments/comment-form.tsx @@ -0,0 +1,100 @@ +import { type KeyboardEvent, useEffect, useRef, useState } from "react"; +import { Button } from "@/components/ui/button"; +import { CommentMarkdownEditor } from "./comment-markdown-editor"; + +interface CommentFormProps { + /** Label for the primary submit button (e.g. "Comment", "Reply", "Update"). */ + label: string; + onSubmit: (body: string) => void | Promise; + onCancel: () => void; + placeholder?: string; + error?: string | null; + /** Pre-fill the textarea when editing an existing comment. */ + initialBody?: string; + autoFocus?: boolean; +} + +export function CommentForm({ + label, + onSubmit, + onCancel, + placeholder = "Leave a comment", + error, + initialBody, + autoFocus = true, +}: CommentFormProps) { + const [body, setBody] = useState(initialBody ?? ""); + const [isSubmitting, setIsSubmitting] = useState(false); + const textareaRef = useRef(null); + const submittingRef = useRef(false); + const hasContent = body.trim().length > 0; + + useEffect(() => { + if (!autoFocus) return; + const textarea = textareaRef.current; + if (!textarea) return; + textarea.focus(); + textarea.selectionStart = textarea.value.length; + textarea.selectionEnd = textarea.value.length; + }, [autoFocus]); + + async function runSubmit() { + const trimmed = body.trim(); + if (!trimmed || submittingRef.current) return; + submittingRef.current = true; + setIsSubmitting(true); + try { + await onSubmit(trimmed); + setBody(""); + } catch { + // The caller surfaces the error; preserve the body so the user can retry. + } finally { + submittingRef.current = false; + setIsSubmitting(false); + } + } + + function handleKeyDown(e: KeyboardEvent) { + if (e.key === "Enter" && (e.metaKey || e.ctrlKey)) { + e.preventDefault(); + void runSubmit(); + } + if (e.key === "Escape") { + e.preventDefault(); + onCancel(); + } + } + + return ( +
+ + {error &&

{error}

} +
+ + +
+ +
+ ); +} diff --git a/packages/web/src/components/comments/comment-markdown-editor.tsx b/packages/web/src/components/comments/comment-markdown-editor.tsx new file mode 100644 index 0000000..3698f4b --- /dev/null +++ b/packages/web/src/components/comments/comment-markdown-editor.tsx @@ -0,0 +1,97 @@ +import { type KeyboardEvent, type ReactNode, type RefObject, useState } from "react"; +import TextareaAutosize from "react-textarea-autosize"; +import { Markdown } from "@/components/ui/markdown"; +import { cn } from "@/lib/utils"; + +const EDITOR_MODE = { + WRITE: "write", + PREVIEW: "preview", +} as const; +type EditorMode = (typeof EDITOR_MODE)[keyof typeof EDITOR_MODE]; +const EDITOR_MODE_LABEL: Record = { + [EDITOR_MODE.WRITE]: "Write", + [EDITOR_MODE.PREVIEW]: "Preview", +}; + +interface CommentMarkdownEditorProps { + value: string; + onChange: (value: string) => void; + textareaRef: RefObject; + placeholder: string; + disabled?: boolean; + minRows?: number; + maxRows?: number; + className?: string; + textareaClassName?: string; + previewClassName?: string; + onKeyDown?: (event: KeyboardEvent) => void; + children?: ReactNode; +} + +export function CommentMarkdownEditor({ + value, + onChange, + textareaRef, + placeholder, + disabled = false, + minRows = 2, + maxRows, + className, + textareaClassName, + previewClassName, + onKeyDown, + children, +}: CommentMarkdownEditorProps) { + const [mode, setMode] = useState(EDITOR_MODE.WRITE); + + return ( +
+
+ {Object.values(EDITOR_MODE).map((nextMode) => { + const isActive = mode === nextMode; + return ( + + ); + })} +
+
+ onChange(event.target.value)} + placeholder={placeholder} + disabled={disabled} + minRows={minRows} + maxRows={maxRows} + className={cn( + "w-full resize-none bg-transparent text-sm placeholder:text-muted-foreground focus:outline-none disabled:opacity-50", + mode === EDITOR_MODE.PREVIEW && "hidden", + textareaClassName, + )} + /> + {mode === EDITOR_MODE.PREVIEW && + (value.trim().length > 0 ? ( + + ) : ( +

+ Nothing to preview +

+ ))} + {children} +
+
+ ); +} diff --git a/packages/web/src/components/comments/comment-thread.tsx b/packages/web/src/components/comments/comment-thread.tsx new file mode 100644 index 0000000..9c3df81 --- /dev/null +++ b/packages/web/src/components/comments/comment-thread.tsx @@ -0,0 +1,323 @@ +import { ChevronRight, Circle, CircleCheck, MessageSquare, User } from "lucide-react"; +import { useState } from "react"; +import { + AlertDialog, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@/components/ui/alert-dialog"; +import { Avatar, AvatarFallback } from "@/components/ui/avatar"; +import { Button } from "@/components/ui/button"; +import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@/components/ui/collapsible"; +import { Markdown } from "@/components/ui/markdown"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; +import { useCommentThreadsContext } from "@/lib/comment-threads-context"; +import { formatTimeAgo } from "@/lib/format"; +import type { Comment, CommentThread } from "@/lib/use-comment-threads"; +import { cn } from "@/lib/utils"; +import { CommentActions } from "./comment-actions"; +import { CommentForm } from "./comment-form"; + +type DeleteTarget = + | { kind: "thread"; hasReplies: boolean } + | { kind: "comment"; commentId: string }; + +function errorMessage(err: unknown, fallback: string): string { + return err instanceof Error ? err.message : fallback; +} + +export function CommentThreadView({ thread }: { thread: CommentThread }) { + const { replyToThread, setThreadResolved, editComment, deleteThread, deleteComment } = + useCommentThreadsContext(); + const isResolved = thread.resolvedAt !== null; + + const [isOpen, setIsOpen] = useState(!isResolved); + const [isReplying, setIsReplying] = useState(false); + const [editingId, setEditingId] = useState(null); + const [deleteTarget, setDeleteTarget] = useState(null); + const [error, setError] = useState(null); + + const root = thread.comments[0]; + // A thread always has a root comment (deleting the last one removes the thread), + // but noUncheckedIndexedAccess types the lookup as possibly-undefined. + if (!root) return null; + const replies = thread.comments.slice(1); + + function handleResolveToggle() { + const next = !isResolved; + setIsOpen(!next); // collapse on resolve, expand on reopen + void setThreadResolved({ threadId: thread.id, resolved: next }); + } + + function handleOpenChange(open: boolean) { + // Keep the thread expanded while the user is mid-action. + if (!open && (isReplying || editingId !== null || deleteTarget !== null)) return; + setIsOpen(open); + } + + async function submitReply(body: string) { + setError(null); + try { + await replyToThread({ threadId: thread.id, body }); + setIsReplying(false); + } catch (err) { + setError(errorMessage(err, "Failed to add reply")); + throw err; + } + } + + async function submitEdit(commentId: string, body: string) { + setError(null); + try { + await editComment({ commentId, body }); + setEditingId(null); + } catch (err) { + setError(errorMessage(err, "Failed to update comment")); + throw err; + } + } + + function confirmDelete() { + if (!deleteTarget) return; + if (deleteTarget.kind === "thread") void deleteThread(thread.id); + else void deleteComment(deleteTarget.commentId); + setDeleteTarget(null); + } + + const idle = !isReplying && editingId === null; + + return ( + +
+
+ + + + + + + {isOpen ? "Collapse thread" : "Expand thread"} + + + + {idle && ( +
+ + + + + Reply + + { + setIsOpen(true); + setError(null); + setEditingId(root.id); + }} + onDelete={() => setDeleteTarget({ kind: "thread", hasReplies: replies.length > 0 })} + deleteLabel={replies.length > 0 ? "Delete thread" : "Delete"} + /> +
+ )} +
+ + + {editingId === root.id ? ( + submitEdit(root.id, b)} + onCancel={() => { + setEditingId(null); + setError(null); + }} + /> + ) : ( + + )} + + {replies.length > 0 && ( +
+ {replies.map((reply) => ( + { + setError(null); + setEditingId(reply.id); + }} + onCancelEdit={() => { + setEditingId(null); + setError(null); + }} + onSubmitEdit={(b) => submitEdit(reply.id, b)} + onDelete={() => setDeleteTarget({ kind: "comment", commentId: reply.id })} + /> + ))} +
+ )} + + {isReplying && ( + { + setIsReplying(false); + setError(null); + }} + /> + )} + +
+ + setDeleteTarget(null)} + onConfirm={confirmDelete} + /> + + ); +} + +function ResolveButton({ isResolved, onToggle }: { isResolved: boolean; onToggle: () => void }) { + return ( + + + + + {isResolved ? "Reopen conversation" : "Mark as resolved"} + + ); +} + +function CommentByline({ createdAt }: { createdAt: string }) { + return ( +

+ + + + + + You + +

+ ); +} + +function ReplyItem({ + reply, + isEditing, + error, + onEdit, + onCancelEdit, + onSubmitEdit, + onDelete, +}: { + reply: Comment; + isEditing: boolean; + error: string | null; + onEdit: () => void; + onCancelEdit: () => void; + onSubmitEdit: (body: string) => Promise; + onDelete: () => void; +}) { + return ( +
+
+ + {!isEditing && } +
+ {isEditing ? ( + + ) : ( + + )} +
+ ); +} + +function DeleteDialog({ + target, + onCancel, + onConfirm, +}: { + target: DeleteTarget | null; + onCancel: () => void; + onConfirm: () => void; +}) { + const isThreadDelete = target?.kind === "thread" && target.hasReplies; + return ( + { + if (!open) onCancel(); + }} + > + + + {isThreadDelete ? "Delete thread" : "Delete comment"} + + {isThreadDelete + ? "This deletes the whole conversation, including replies. This can't be undone." + : "This deletes the comment. This can't be undone."} + + + + Cancel + + + + + ); +} From 0c76d0fcef809901ad1bc16a0d9063dc585887a0 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:34:33 -0700 Subject: [PATCH 06/30] feat(PRO-546): render and create line-anchored comments in the diff viewer --- .../components/chapter/pierre-diff-viewer.tsx | 184 +++++++++++++++++- .../chapter/text-selection-popup.tsx | 34 +++- 2 files changed, 212 insertions(+), 6 deletions(-) diff --git a/packages/web/src/components/chapter/pierre-diff-viewer.tsx b/packages/web/src/components/chapter/pierre-diff-viewer.tsx index 278a2a8..6e6b0a4 100644 --- a/packages/web/src/components/chapter/pierre-diff-viewer.tsx +++ b/packages/web/src/components/chapter/pierre-diff-viewer.tsx @@ -1,21 +1,39 @@ import { + type DiffLineAnnotation, type FileDiffMetadata, + type GetHoveredLineResult, getSingularPatch, type Hunk, type SelectedLineRange, } from "@pierre/diffs"; import { FileDiff, PatchDiff } from "@pierre/diffs/react"; -import { useDeferredValue, useEffect, useMemo, useRef, useState } from "react"; +import { Plus } from "lucide-react"; +import { + type ReactNode, + useCallback, + useDeferredValue, + useEffect, + useMemo, + useRef, + useState, +} from "react"; +import { CommentForm } from "@/components/comments/comment-form"; +import { CommentThreadView } from "@/components/comments/comment-thread"; +import { useCommentThreadsContext } from "@/lib/comment-threads-context"; import { type AnnotatedLineRef, COMMENT_SIDE, DIFF_SIDE, + type DiffSide, type LineRef, SIDE_TO_DIFF, } from "@/lib/diff-types"; import { resolveSyntaxTheme } from "@/lib/syntax-themes"; +import type { CommentThread } from "@/lib/use-comment-threads"; import { useDiffSettings } from "@/lib/use-diff-settings"; +import { useTextSelection } from "@/lib/use-text-selection"; import { LineHighlightOverlay } from "./hunk-highlight-overlay"; +import { TextSelectionPopup } from "./text-selection-popup"; type AppTheme = "light" | "dark"; @@ -80,6 +98,41 @@ export function getVisibleLineRange( }; } +interface CommentDraft { + side: DiffSide; + startLine: number; + endLine: number; +} + +// Groups threads (plus any in-progress draft) by their anchor line so Pierre +// renders one annotation row per (side, line) directly below the diff line. +function buildCommentAnnotations( + threads: CommentThread[], + draft: CommentDraft | null, +): DiffLineAnnotation[] { + const bySideLine = new Map>>(); + const ensure = (side: DiffSide, line: number): DiffLineAnnotation => { + let byLine = bySideLine.get(side); + if (!byLine) { + byLine = new Map(); + bySideLine.set(side, byLine); + } + let entry = byLine.get(line); + if (!entry) { + entry = { side, lineNumber: line, metadata: [] }; + byLine.set(line, entry); + } + return entry; + }; + for (const thread of threads) ensure(thread.side, thread.endLine).metadata.push(thread); + if (draft) ensure(draft.side, draft.endLine); + const out: DiffLineAnnotation[] = []; + for (const byLine of bySideLine.values()) { + for (const entry of byLine.values()) out.push(entry); + } + return out; +} + type PierreDiffViewerProps = { filePath?: string; selectedLines?: SelectedLineRange | null; @@ -142,6 +195,114 @@ export function PierreDiffViewer({ return allLineRefsByFile.get(filePath); }, [allLineRefsByFile, filePath]); + // ---- Line-anchored comments ---- + const comments = useCommentThreadsContext(); + const { createThread } = comments; + const fileThreads = useMemo( + () => (filePath ? (comments.threadsByFile.get(filePath) ?? []) : []), + [comments.threadsByFile, filePath], + ); + // The line range the user is composing a new comment on (null when idle). + const [draft, setDraft] = useState(null); + const [draftError, setDraftError] = useState(null); + const { selectionInfo, clearSelection } = useTextSelection(diffContainerRef); + + const lineAnnotations = useMemo( + () => buildCommentAnnotations(fileThreads, draft), + [fileThreads, draft], + ); + + const cancelDraft = useCallback(() => { + setDraft(null); + setDraftError(null); + }, []); + + const handleCreateComment = useCallback( + async (body: string) => { + if (!draft || !filePath) return; + setDraftError(null); + try { + await createThread({ + filePath, + side: draft.side, + startLine: draft.startLine, + endLine: draft.endLine, + body, + }); + setDraft(null); + } catch (err) { + setDraftError(err instanceof Error ? err.message : "Failed to add comment"); + throw err; // keep the composer open with the body intact + } + }, + [draft, filePath, createThread], + ); + + const renderAnnotation = useCallback( + (annotation: DiffLineAnnotation): ReactNode => { + const threads = annotation.metadata ?? []; + const showComposer = + draft !== null && annotation.side === draft.side && annotation.lineNumber === draft.endLine; + if (threads.length === 0 && !showComposer) return null; + return ( +
+ {threads.map((thread) => ( + + ))} + {showComposer && ( + + )} +
+ ); + }, + [draft, draftError, handleCreateComment, cancelDraft], + ); + + const renderGutterUtility = useCallback( + (getHoveredLine: () => GetHoveredLineResult<"diff"> | undefined): ReactNode => ( + + ), + [], + ); + + const handleCommentFromSelection = useCallback( + (range: SelectedLineRange) => { + setDraftError(null); + setDraft({ + side: range.side ?? DIFF_SIDE.ADDITIONS, + startLine: range.start, + endLine: range.end, + }); + clearSelection(); + }, + [clearSelection], + ); + const options = useMemo( () => ({ theme: resolveSyntaxTheme(deferredSyntaxTheme, appTheme), @@ -156,7 +317,7 @@ export function PierreDiffViewer({ expansionLineCount: 20, overflow: deferredWrap ? ("wrap" as const) : ("scroll" as const), enableLineSelection: true, - enableHoverUtility: false, + enableGutterUtility: true, }), [ appTheme, @@ -174,6 +335,9 @@ export function PierreDiffViewer({ const sharedProps = { options, selectedLines: selectedLinesProp ?? null, + lineAnnotations, + renderAnnotation, + renderGutterUtility, }; // Only mount the overlay when this file actually has refs to highlight. @@ -194,14 +358,25 @@ export function PierreDiffViewer({ /> ) : null; + // Only show the popup when text is selected and no composer is already open. + const textSelectionPopup = + selectionInfo && draft === null ? ( + + ) : null; + if (fileDiff) { return (
- + fileDiff={fileDiff} {...sharedProps} /> {overlay} + {textSelectionPopup}
); } @@ -211,8 +386,9 @@ export function PierreDiffViewer({ className="@container/diff relative isolate overflow-hidden rounded-b-lg border-x border-b border-border" ref={diffContainerRef} > - + patch={patch} {...sharedProps} /> {overlay} + {textSelectionPopup}
); } diff --git a/packages/web/src/components/chapter/text-selection-popup.tsx b/packages/web/src/components/chapter/text-selection-popup.tsx index d85209b..58e6d35 100644 --- a/packages/web/src/components/chapter/text-selection-popup.tsx +++ b/packages/web/src/components/chapter/text-selection-popup.tsx @@ -1,4 +1,7 @@ import type { SelectedLineRange } from "@pierre/diffs"; +import { MessageSquarePlus } from "lucide-react"; +import { createPortal } from "react-dom"; +import { Button } from "@/components/ui/button"; interface TextSelectionPopupProps { selectionRect: DOMRect; @@ -6,6 +9,33 @@ interface TextSelectionPopupProps { onComment: (lineRange: SelectedLineRange) => void; } -export function TextSelectionPopup(_props: TextSelectionPopupProps) { - return null; +export function TextSelectionPopup({ + selectionRect, + lineRange, + onComment, +}: TextSelectionPopupProps) { + // Portaled to the body so the diff's overflow:hidden can't clip it. The rect + // is already in page coordinates (the selection hook adds scroll offsets). + return createPortal( +
+ +
, + document.body, + ); } From d85ae76a118c2dc7f7d460a10800ebaf83cbb8e0 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:35:57 -0700 Subject: [PATCH 07/30] test(PRO-546): cover text-selection line-range helpers --- .../lib/__tests__/use-text-selection.test.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 packages/web/src/lib/__tests__/use-text-selection.test.ts diff --git a/packages/web/src/lib/__tests__/use-text-selection.test.ts b/packages/web/src/lib/__tests__/use-text-selection.test.ts new file mode 100644 index 0000000..1d3b2a6 --- /dev/null +++ b/packages/web/src/lib/__tests__/use-text-selection.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; +import { buildSelectedLineRange, normalizeSelectedLineRange } from "../use-text-selection"; + +describe("buildSelectedLineRange", () => { + it("orders endpoints ascending and keeps the side", () => { + expect( + buildSelectedLineRange({ + startLine: 10, + endLine: 5, + startSide: "additions", + endSide: "additions", + }), + ).toEqual({ start: 5, side: "additions", end: 10, endSide: "additions" }); + }); + + it("returns null for a selection spanning both diff sides", () => { + expect( + buildSelectedLineRange({ + startLine: 1, + endLine: 3, + startSide: "deletions", + endSide: "additions", + }), + ).toBeNull(); + }); +}); + +describe("normalizeSelectedLineRange", () => { + it("leaves an already-ascending range unchanged", () => { + const range = { start: 2, side: "additions", end: 6, endSide: "additions" } as const; + expect(normalizeSelectedLineRange(range)).toEqual(range); + }); + + it("swaps endpoints and sides when start > end (drag upward)", () => { + expect( + normalizeSelectedLineRange({ start: 9, side: "additions", end: 4, endSide: "deletions" }), + ).toEqual({ start: 4, side: "deletions", end: 9, endSide: "additions" }); + }); +}); From b2a69dc8812b3e67fe3811a6c0ed80cd098d1ca8 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:42:20 -0700 Subject: [PATCH 08/30] chore(PRO-546): lock react-textarea-autosize dependency --- pnpm-lock.yaml | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c3837ca..b5e6ea7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -160,6 +160,9 @@ importers: react-markdown: specifier: ^10.1.0 version: 10.1.0(@types/react@19.2.14)(react@19.2.5) + react-textarea-autosize: + specifier: ^8.5.9 + version: 8.5.9(@types/react@19.2.14)(react@19.2.5) react-zoom-pan-pinch: specifier: ^3.7.0 version: 3.7.0(react-dom@19.2.5(react@19.2.5))(react@19.2.5) @@ -3555,6 +3558,12 @@ packages: '@types/react': optional: true + react-textarea-autosize@8.5.9: + resolution: {integrity: sha512-U1DGlIQN5AwgjTyOEnI1oCcMuEr1pv1qOtklB2l4nyMGbHzWrI0eFsYK0zos2YWqAolJyG0IWJaqWmWj5ETh0A==} + engines: {node: '>=10'} + peerDependencies: + react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + react-zoom-pan-pinch@3.7.0: resolution: {integrity: sha512-UmReVZ0TxlKzxSbYiAj+LeGRW8s8LraAFTXRAxzMYnNRgGPsxCudwZKVkjvGmjtx7SW/hZamt69NUmGf4xrkXA==} engines: {node: '>=8', npm: '>=5'} @@ -3917,6 +3926,33 @@ packages: '@types/react': optional: true + use-composed-ref@1.4.0: + resolution: {integrity: sha512-djviaxuOOh7wkj0paeO1Q/4wMZ8Zrnag5H6yBvzN7AKKe8beOaED9SF5/ByLqsku8NP4zQqsvM2u3ew/tJK8/w==} + peerDependencies: + '@types/react': '*' + react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + peerDependenciesMeta: + '@types/react': + optional: true + + use-isomorphic-layout-effect@1.2.1: + resolution: {integrity: sha512-tpZZ+EX0gaghDAiFR37hj5MgY6ZN55kLiPkJsKxBMZ6GZdOSPJXiOzPM984oPYZ5AnehYx5WQp1+ME8I/P/pRA==} + peerDependencies: + '@types/react': '*' + react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + peerDependenciesMeta: + '@types/react': + optional: true + + use-latest@1.3.0: + resolution: {integrity: sha512-mhg3xdm9NaM8q+gLT8KryJPnRFOz1/5XPBhmDEVZK1webPzDjrPk7f/mbpeLqTgB9msytYWANxgALOCJKnLvcQ==} + peerDependencies: + '@types/react': '*' + react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + peerDependenciesMeta: + '@types/react': + optional: true + use-sidecar@1.1.3: resolution: {integrity: sha512-Fedw0aZvkhynoPYlA5WXrMCAMm+nSWdZt6lzJQ7Ok8S6Q+VsHmHpRWndVRJ8Be0ZbkfPc5LRYH+5XrzXcEeLRQ==} engines: {node: '>=10'} @@ -7481,6 +7517,15 @@ snapshots: optionalDependencies: '@types/react': 19.2.14 + react-textarea-autosize@8.5.9(@types/react@19.2.14)(react@19.2.5): + dependencies: + '@babel/runtime': 7.29.2 + react: 19.2.5 + use-composed-ref: 1.4.0(@types/react@19.2.14)(react@19.2.5) + use-latest: 1.3.0(@types/react@19.2.14)(react@19.2.5) + transitivePeerDependencies: + - '@types/react' + react-zoom-pan-pinch@3.7.0(react-dom@19.2.5(react@19.2.5))(react@19.2.5): dependencies: react: 19.2.5 @@ -7903,6 +7948,25 @@ snapshots: optionalDependencies: '@types/react': 19.2.14 + use-composed-ref@1.4.0(@types/react@19.2.14)(react@19.2.5): + dependencies: + react: 19.2.5 + optionalDependencies: + '@types/react': 19.2.14 + + use-isomorphic-layout-effect@1.2.1(@types/react@19.2.14)(react@19.2.5): + dependencies: + react: 19.2.5 + optionalDependencies: + '@types/react': 19.2.14 + + use-latest@1.3.0(@types/react@19.2.14)(react@19.2.5): + dependencies: + react: 19.2.5 + use-isomorphic-layout-effect: 1.2.1(@types/react@19.2.14)(react@19.2.5) + optionalDependencies: + '@types/react': 19.2.14 + use-sidecar@1.1.3(@types/react@19.2.14)(react@19.2.5): dependencies: detect-node-es: 1.1.0 From 2a15ad143a694603cc1ce0716d82bf3b3bb490a9 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:57:00 -0700 Subject: [PATCH 09/30] fix(PRO-546): match hosted gutter comment button (blue, no scale) --- .../components/chapter/pierre-diff-viewer.tsx | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/packages/web/src/components/chapter/pierre-diff-viewer.tsx b/packages/web/src/components/chapter/pierre-diff-viewer.tsx index 6e6b0a4..0c86746 100644 --- a/packages/web/src/components/chapter/pierre-diff-viewer.tsx +++ b/packages/web/src/components/chapter/pierre-diff-viewer.tsx @@ -269,23 +269,25 @@ export function PierreDiffViewer({ const renderGutterUtility = useCallback( (getHoveredLine: () => GetHoveredLineResult<"diff"> | undefined): ReactNode => ( - +
+ +
), [], ); From cf10157ba3e95b6d8dde794e1ef7ed5286d6738c Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 17:14:56 -0700 Subject: [PATCH 10/30] fix(PRO-546): use literal styles for gutter + button (Tailwind vars don't reach Pierre's slot) --- .../components/chapter/pierre-diff-viewer.tsx | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/packages/web/src/components/chapter/pierre-diff-viewer.tsx b/packages/web/src/components/chapter/pierre-diff-viewer.tsx index 0c86746..be2b4ec 100644 --- a/packages/web/src/components/chapter/pierre-diff-viewer.tsx +++ b/packages/web/src/components/chapter/pierre-diff-viewer.tsx @@ -9,6 +9,7 @@ import { import { FileDiff, PatchDiff } from "@pierre/diffs/react"; import { Plus } from "lucide-react"; import { + type CSSProperties, type ReactNode, useCallback, useDeferredValue, @@ -153,6 +154,27 @@ type PierreDiffViewerProps = { const noop = () => {}; const noopChecked = () => false; +// Literal styles for the hover "+" — see renderGutterUtility for why Tailwind +// utilities can't be used here. `backgroundColor` is Tailwind blue-500's value. +const GUTTER_SLOT_STYLE: CSSProperties = { + display: "flex", + height: "100%", + alignItems: "flex-start", + justifyContent: "center", + paddingTop: "2px", +}; +const GUTTER_BUTTON_STYLE: CSSProperties = { + display: "flex", + alignItems: "center", + justifyContent: "center", + width: "16px", + height: "16px", + borderRadius: "4px", + backgroundColor: "oklch(62.3% 0.214 259.815)", + color: "#fff", + cursor: "pointer", +}; + export function PierreDiffViewer({ patch, fileDiff, @@ -269,11 +291,16 @@ export function PierreDiffViewer({ const renderGutterUtility = useCallback( (getHoveredLine: () => GetHoveredLineResult<"diff"> | undefined): ReactNode => ( -
+ // Pierre projects this into its shadow DOM via a , and slotted content + // inherits custom properties from the shadow tree, not the light-DOM :root. + // Tailwind v4 utilities resolve through `--color-*`/`--spacing`/`--radius` + // vars that aren't defined there, so they'd compute to transparent/zero. + // Style with literal values (blue-500 = the resolved `--color-blue-500`). +
), From f1d7a388f70009e78f0424d619a2b06cead97b0d Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 17:37:45 -0700 Subject: [PATCH 11/30] fix(PRO-546): match hosted text-selection comment popup (dark toolbar) --- .../chapter/text-selection-popup.tsx | 72 ++++++++++++++----- 1 file changed, 55 insertions(+), 17 deletions(-) diff --git a/packages/web/src/components/chapter/text-selection-popup.tsx b/packages/web/src/components/chapter/text-selection-popup.tsx index 58e6d35..77553f8 100644 --- a/packages/web/src/components/chapter/text-selection-popup.tsx +++ b/packages/web/src/components/chapter/text-selection-popup.tsx @@ -1,40 +1,78 @@ import type { SelectedLineRange } from "@pierre/diffs"; -import { MessageSquarePlus } from "lucide-react"; +import { MessageSquare } from "lucide-react"; +import { useLayoutEffect, useRef } from "react"; import { createPortal } from "react-dom"; -import { Button } from "@/components/ui/button"; interface TextSelectionPopupProps { + /** Bounding rect of the text selection in page (document) coordinates. */ selectionRect: DOMRect; lineRange: SelectedLineRange; onComment: (lineRange: SelectedLineRange) => void; } +/** + * Clamp the popup's anchor so it stays within the viewport after the CSS + * `translate(-50%)`. Measured in a layout effect since the width is unknown + * until rendered. + */ +function clampHorizontally(popup: HTMLElement, anchorLeft: number) { + const halfWidth = popup.offsetWidth / 2; + const minLeft = window.scrollX + halfWidth + 8; + const maxLeft = window.innerWidth + window.scrollX - halfWidth - 8; + popup.style.left = `${Math.max(minLeft, Math.min(anchorLeft, maxLeft))}px`; +} + +const BUTTON_CLASS = + "flex h-7 items-center gap-1.5 rounded px-2.5 text-xs font-medium text-zinc-100 transition-colors hover:bg-white/10"; + export function TextSelectionPopup({ selectionRect, lineRange, onComment, }: TextSelectionPopupProps) { - // Portaled to the body so the diff's overflow:hidden can't clip it. The rect - // is already in page coordinates (the selection hook adds scroll offsets). + const popupRef = useRef(null); + + // selectionRect is already in page coordinates; `translate(-50%, -100%)` + // centers the popup horizontally and floats it just above the selection. + const top = selectionRect.top - 8; + const left = selectionRect.left + selectionRect.width / 2; + + useLayoutEffect(() => { + const popup = popupRef.current; + if (!popup) return; + clampHorizontally(popup, left); + }, [left]); + + const isMultiLine = lineRange.start !== lineRange.end; + const rangeLabel = isMultiLine + ? `lines ${lineRange.start}–${lineRange.end}` + : `line ${lineRange.start}`; + return createPortal(
- +
+ +
, document.body, ); From 732b46614ab888faecdf2668b14fc9a117f688d3 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 18:07:56 -0700 Subject: [PATCH 12/30] fix(PRO-546): add markdown formatting toolbar to the comment editor --- .../comments/comment-markdown-editor.tsx | 51 +-- .../components/comments/markdown-toolbar.tsx | 303 ++++++++++++++++++ 2 files changed, 334 insertions(+), 20 deletions(-) create mode 100644 packages/web/src/components/comments/markdown-toolbar.tsx diff --git a/packages/web/src/components/comments/comment-markdown-editor.tsx b/packages/web/src/components/comments/comment-markdown-editor.tsx index 3698f4b..928f23f 100644 --- a/packages/web/src/components/comments/comment-markdown-editor.tsx +++ b/packages/web/src/components/comments/comment-markdown-editor.tsx @@ -2,6 +2,7 @@ import { type KeyboardEvent, type ReactNode, type RefObject, useState } from "re import TextareaAutosize from "react-textarea-autosize"; import { Markdown } from "@/components/ui/markdown"; import { cn } from "@/lib/utils"; +import { MarkdownToolbar } from "./markdown-toolbar"; const EDITOR_MODE = { WRITE: "write", @@ -46,26 +47,36 @@ export function CommentMarkdownEditor({ return (
-
- {Object.values(EDITOR_MODE).map((nextMode) => { - const isActive = mode === nextMode; - return ( - - ); - })} +
+
+ {Object.values(EDITOR_MODE).map((nextMode) => { + const isActive = mode === nextMode; + return ( + + ); + })} +
+ {/* Suggestion blocks only apply on a PR, so omit that toolbar item for local comments. */} +
; + label: string; + apply: (ctx: SelectionContext) => InsertResult; + visibleFromClass?: string; +} + +interface SelectionContext { + value: string; + selectionStart: number; + selectionEnd: number; +} + +interface InsertResult { + value: string; + selectionStart: number; + selectionEnd: number; +} + +function wrapSelection( + ctx: SelectionContext, + prefix: string, + suffix: string, + placeholder: string, +): InsertResult { + const { value, selectionStart, selectionEnd } = ctx; + const selected = value.slice(selectionStart, selectionEnd); + const before = value.slice(0, selectionStart); + const after = value.slice(selectionEnd); + + if (selected) { + const wrapped = `${prefix}${selected}${suffix}`; + return { + value: `${before}${wrapped}${after}`, + selectionStart: selectionStart + prefix.length, + selectionEnd: selectionStart + prefix.length + selected.length, + }; + } + + const inserted = `${prefix}${placeholder}${suffix}`; + return { + value: `${before}${inserted}${after}`, + selectionStart: selectionStart + prefix.length, + selectionEnd: selectionStart + prefix.length + placeholder.length, + }; +} + +function prefixLine(ctx: SelectionContext, prefix: string, placeholder: string): InsertResult { + const { value, selectionStart, selectionEnd } = ctx; + const selected = value.slice(selectionStart, selectionEnd); + const before = value.slice(0, selectionStart); + const after = value.slice(selectionEnd); + + const needsNewline = before.length > 0 && !before.endsWith("\n"); + const needsTrailingNewline = after.length > 0 && !after.startsWith("\n"); + const linePrefix = needsNewline ? `\n${prefix}` : prefix; + const trailingNewline = needsTrailingNewline ? "\n" : ""; + + if (selected) { + const lines = selected.split("\n"); + if (lines.length > 1 && lines[lines.length - 1] === "") { + lines.pop(); + } + const prefixed = lines.map((line) => `${prefix}${line}`).join("\n"); + const inserted = `${needsNewline ? "\n" : ""}${prefixed}`; + return { + value: `${before}${inserted}${trailingNewline}${after}`, + selectionStart: selectionStart + (needsNewline ? 1 : 0), + selectionEnd: selectionStart + inserted.length, + }; + } + + const inserted = `${linePrefix}${placeholder}`; + return { + value: `${before}${inserted}${trailingNewline}${after}`, + selectionStart: selectionStart + linePrefix.length, + selectionEnd: selectionStart + linePrefix.length + placeholder.length, + }; +} + +function insertBlock( + ctx: SelectionContext, + openFence: string, + closeFence: string, + placeholder: string, +): InsertResult { + const { value, selectionStart, selectionEnd } = ctx; + const selected = value.slice(selectionStart, selectionEnd); + const before = value.slice(0, selectionStart); + const after = value.slice(selectionEnd); + + const needsNewline = before.length > 0 && !before.endsWith("\n"); + const needsTrailingNewline = after.length > 0 && !after.startsWith("\n"); + const blockPrefix = needsNewline ? `\n${openFence}\n` : `${openFence}\n`; + const rawContent = selected || placeholder; + const content = rawContent.endsWith("\n") ? rawContent.slice(0, -1) : rawContent; + const block = `${blockPrefix}${content}\n${closeFence}`; + const trailingNewline = needsTrailingNewline ? "\n" : ""; + + return { + value: `${before}${block}${trailingNewline}${after}`, + selectionStart: selectionStart + blockPrefix.length, + selectionEnd: selectionStart + blockPrefix.length + content.length, + }; +} + +interface ToolbarSeparator { + key: string; + visibleFromClass?: string; +} + +type ToolbarItem = ToolbarAction | ToolbarSeparator; + +const TOOLBAR_ITEMS: ToolbarItem[] = [ + { + icon: FileDiff, + label: "Suggestion", + apply: (ctx) => insertBlock(ctx, "```suggestion", "```", "suggestion"), + }, + { + icon: Heading2, + label: "Heading", + apply: (ctx) => prefixLine(ctx, "## ", "heading"), + visibleFromClass: "hidden @[13rem]:inline-flex", + }, + { + icon: Bold, + label: "Bold", + apply: (ctx) => wrapSelection(ctx, "**", "**", "bold text"), + }, + { + icon: Italic, + label: "Italic", + apply: (ctx) => wrapSelection(ctx, "_", "_", "italic text"), + }, + { + icon: TextQuote, + label: "Quote", + apply: (ctx) => prefixLine(ctx, "> ", "quote"), + visibleFromClass: "hidden @[15rem]:inline-flex", + }, + { + icon: Code, + label: "Code", + apply: (ctx) => wrapSelection(ctx, "`", "`", "code"), + }, + { + icon: Link, + label: "Link", + apply: (ctx) => { + const { value, selectionStart, selectionEnd } = ctx; + const selected = value.slice(selectionStart, selectionEnd); + const before = value.slice(0, selectionStart); + const after = value.slice(selectionEnd); + + if (selected) { + const inserted = `[${selected}](url)`; + return { + value: `${before}${inserted}${after}`, + selectionStart: selectionStart + selected.length + 3, + selectionEnd: selectionStart + selected.length + 6, + }; + } + + const inserted = "[link text](url)"; + return { + value: `${before}${inserted}${after}`, + selectionStart: selectionStart + 1, + selectionEnd: selectionStart + 10, + }; + }, + }, + { key: "list-separator", visibleFromClass: "hidden @[17rem]:block" }, + { + icon: List, + label: "Bulleted list", + apply: (ctx) => prefixLine(ctx, "- ", "list item"), + visibleFromClass: "hidden @[17rem]:inline-flex", + }, + { + icon: ListOrdered, + label: "Numbered list", + apply: (ctx) => prefixLine(ctx, "1. ", "list item"), + visibleFromClass: "hidden @[19rem]:inline-flex", + }, + { + icon: ListChecks, + label: "Task list", + apply: (ctx) => prefixLine(ctx, "- [ ] ", "task"), + visibleFromClass: "hidden @[21rem]:inline-flex", + }, +]; + +interface MarkdownToolbarProps { + textareaRef: RefObject; + onChange: (value: string) => void; + showSuggestion?: boolean; + disabled?: boolean; + className?: string; +} + +export function MarkdownToolbar({ + textareaRef, + onChange, + showSuggestion = true, + disabled = false, + className, +}: MarkdownToolbarProps) { + // Suppress tooltips briefly after mount so they don't flash when a parent + // popover opens beneath the cursor. + const [tooltipsReady, setTooltipsReady] = useState(false); + useEffect(() => { + const id = setTimeout(() => setTooltipsReady(true), 400); + return () => clearTimeout(id); + }, []); + + function handleAction(action: ToolbarAction) { + const textarea = textareaRef.current; + if (!textarea) return; + + const ctx: SelectionContext = { + value: textarea.value, + selectionStart: textarea.selectionStart, + selectionEnd: textarea.selectionEnd, + }; + + const result = action.apply(ctx); + onChange(result.value); + + requestAnimationFrame(() => { + textarea.focus(); + textarea.setSelectionRange(result.selectionStart, result.selectionEnd); + }); + } + + return ( +
+ {TOOLBAR_ITEMS.filter( + (item) => showSuggestion || !("label" in item && item.label === "Suggestion"), + ).map((item) => { + if (!("icon" in item)) { + return ( + + ); +} From 39c790556fd24b191e31963cbe9eb4f74f9f20a6 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 19:41:43 -0700 Subject: [PATCH 13/30] fix(PRO-546): resolve Pierre shadow root lazily for text selection Pierre creates its shadow root asynchronously, so caching it once on mount left getSelection() reading a null root when the diff rendered late, silently breaking select-to-comment. Resolve it per-event and bind shadow listeners as soon as the root appears (bounded poll), with the container capture listeners as fallback. Addresses Cursor Bugbot finding. --- packages/web/src/lib/use-text-selection.ts | 46 ++++++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/packages/web/src/lib/use-text-selection.ts b/packages/web/src/lib/use-text-selection.ts index 5bb5177..54be8ec 100644 --- a/packages/web/src/lib/use-text-selection.ts +++ b/packages/web/src/lib/use-text-selection.ts @@ -132,12 +132,15 @@ export function useTextSelection(containerRef: React.RefObject). Events from inside - // the shadow root don't reach capture listeners on the outer div, so listen - // on the shadow root directly too. - const diffsContainer = container.querySelector("diffs-container"); - const shadowRoot = diffsContainer?.shadowRoot ?? null; - shadowRootRef.current = shadowRoot; + // Pierre creates its shadow root asynchronously (its + // highlighting is worker-backed), so it may not exist yet when this effect + // first runs. Resolve it lazily on each event instead of caching a + // possibly-null value. + const resolveShadowRoot = (): ShadowRoot | null => { + const root = container.querySelector("diffs-container")?.shadowRoot ?? null; + shadowRootRef.current = root; + return root; + }; const handleMouseDown = () => { isMouseDownRef.current = true; @@ -156,6 +159,7 @@ export function useTextSelection(containerRef: React.RefObject { rafIdRef.current = null; + const shadowRoot = resolveShadowRoot(); const selection = hasGetSelection(shadowRoot) ? shadowRoot.getSelection() : window.getSelection(); @@ -242,18 +246,34 @@ export function useTextSelection(containerRef: React.RefObject { + pollFrame = null; + const root = resolveShadowRoot(); + if (root) { + boundShadowRoot = root; + root.addEventListener("mousedown", handleMouseDown, true); + root.addEventListener("mouseup", handleMouseUp, true); + } else if (framesLeft-- > 0) { + pollFrame = requestAnimationFrame(bindShadowRoot); + } + }; + bindShadowRoot(); return () => { if (rafIdRef.current !== null) cancelAnimationFrame(rafIdRef.current); + if (pollFrame !== null) cancelAnimationFrame(pollFrame); container.removeEventListener("mousedown", handleMouseDown, true); container.removeEventListener("mouseup", handleMouseUp, true); - if (shadowRoot) { - shadowRoot.removeEventListener("mousedown", handleMouseDown, true); - shadowRoot.removeEventListener("mouseup", handleMouseUp, true); + if (boundShadowRoot) { + boundShadowRoot.removeEventListener("mousedown", handleMouseDown, true); + boundShadowRoot.removeEventListener("mouseup", handleMouseUp, true); } }; }, [containerRef]); From 8672275c15aa40a766b71a8700ed92b61e7e9f11 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 19:52:48 -0700 Subject: [PATCH 14/30] feat(PRO-546): gutter drag-select to comment + highlight thread lines on hover Wire Pierre's onLineSelected so dragging across the line-number gutter opens the composer for the whole range (multi-line comments). Hovering a thread now highlights its anchored lines via selectedLines; an isHoveringRef guard keeps that highlight from triggering onLineSelected and opening a composer. --- .../components/chapter/pierre-diff-viewer.tsx | 60 +++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/packages/web/src/components/chapter/pierre-diff-viewer.tsx b/packages/web/src/components/chapter/pierre-diff-viewer.tsx index be2b4ec..d46c88d 100644 --- a/packages/web/src/components/chapter/pierre-diff-viewer.tsx +++ b/packages/web/src/components/chapter/pierre-diff-viewer.tsx @@ -32,7 +32,7 @@ import { import { resolveSyntaxTheme } from "@/lib/syntax-themes"; import type { CommentThread } from "@/lib/use-comment-threads"; import { useDiffSettings } from "@/lib/use-diff-settings"; -import { useTextSelection } from "@/lib/use-text-selection"; +import { normalizeSelectedLineRange, useTextSelection } from "@/lib/use-text-selection"; import { LineHighlightOverlay } from "./hunk-highlight-overlay"; import { TextSelectionPopup } from "./text-selection-popup"; @@ -229,6 +229,12 @@ export function PierreDiffViewer({ const [draftError, setDraftError] = useState(null); const { selectionInfo, clearSelection } = useTextSelection(diffContainerRef); + // Hovering a thread highlights its anchored lines. Highlighting sets Pierre's + // `selectedLines`, which makes Pierre fire `onLineSelected` — the ref lets us + // tell that apart from a genuine drag so a hover doesn't open the composer. + const [hoverLines, setHoverLines] = useState(null); + const isHoveringRef = useRef(false); + const lineAnnotations = useMemo( () => buildCommentAnnotations(fileThreads, draft), [fileThreads, draft], @@ -260,6 +266,21 @@ export function PierreDiffViewer({ [draft, filePath, createThread], ); + const handleThreadMouseEnter = useCallback((thread: CommentThread) => { + isHoveringRef.current = true; + setHoverLines({ + start: thread.startLine, + side: thread.side, + end: thread.endLine, + endSide: thread.side, + }); + }, []); + + const handleThreadMouseLeave = useCallback(() => { + isHoveringRef.current = false; + setHoverLines(null); + }, []); + const renderAnnotation = useCallback( (annotation: DiffLineAnnotation): ReactNode => { const threads = annotation.metadata ?? []; @@ -272,7 +293,14 @@ export function PierreDiffViewer({ style={{ maxWidth: "min(48rem, 90cqw)", whiteSpace: "normal" }} > {threads.map((thread) => ( - + // biome-ignore lint/a11y/noStaticElementInteractions: hover only highlights the anchored lines, it's not an interactive control +
handleThreadMouseEnter(thread)} + onMouseLeave={handleThreadMouseLeave} + > + +
))} {showComposer && ( ); }, - [draft, draftError, handleCreateComment, cancelDraft], + [ + draft, + draftError, + handleCreateComment, + cancelDraft, + handleThreadMouseEnter, + handleThreadMouseLeave, + ], ); const renderGutterUtility = useCallback( @@ -332,6 +367,20 @@ export function PierreDiffViewer({ [clearSelection], ); + // Dragging across the line-number gutter selects a range and opens the composer + // for the whole span. Suppressed while hovering a thread, since the highlight + // also changes `selectedLines` and would otherwise trigger this. + const handleLineSelected = useCallback((range: SelectedLineRange | null) => { + if (isHoveringRef.current || !range) return; + const norm = normalizeSelectedLineRange(range); + setDraftError(null); + setDraft({ + side: norm.side ?? DIFF_SIDE.ADDITIONS, + startLine: norm.start, + endLine: norm.end, + }); + }, []); + const options = useMemo( () => ({ theme: resolveSyntaxTheme(deferredSyntaxTheme, appTheme), @@ -347,6 +396,7 @@ export function PierreDiffViewer({ overflow: deferredWrap ? ("wrap" as const) : ("scroll" as const), enableLineSelection: true, enableGutterUtility: true, + onLineSelected: handleLineSelected, }), [ appTheme, @@ -358,12 +408,14 @@ export function PierreDiffViewer({ deferredWrap, deferredLineNumbers, deferredExpandUnchanged, + handleLineSelected, ], ); const sharedProps = { options, - selectedLines: selectedLinesProp ?? null, + // Hover-highlight takes precedence over any parent-controlled selection. + selectedLines: hoverLines ?? selectedLinesProp ?? null, lineAnnotations, renderAnnotation, renderGutterUtility, From 58b02b6a8fa710b577204fe353d501de9002fdb1 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 22:29:53 -0700 Subject: [PATCH 15/30] feat(PRO-546): show the gh-authenticated user as the comment author Resolve the local reviewer's identity via `gh api user` (falling back to git config user.name, then a generic 'You') and render their name + avatar in the comment byline instead of a hardcoded placeholder. Adds a /api/viewer route, a useViewer hook, and the Viewer wire type. Degrades gracefully when gh is unauthenticated or the repo isn't on GitHub. --- .../cli/src/__tests__/viewer.routes.test.ts | 93 +++++++++++++++++++ packages/cli/src/git.ts | 13 +++ packages/cli/src/github/index.ts | 1 + packages/cli/src/github/viewer.ts | 35 +++++++ packages/cli/src/routes/viewer.ts | 28 ++++++ packages/cli/src/show.ts | 2 + packages/types/package.json | 3 +- packages/types/src/index.ts | 1 + packages/types/src/viewer.ts | 9 ++ .../components/comments/comment-thread.tsx | 7 +- packages/web/src/lib/use-viewer.ts | 24 +++++ 11 files changed, 213 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/__tests__/viewer.routes.test.ts create mode 100644 packages/cli/src/github/viewer.ts create mode 100644 packages/cli/src/routes/viewer.ts create mode 100644 packages/types/src/viewer.ts create mode 100644 packages/web/src/lib/use-viewer.ts diff --git a/packages/cli/src/__tests__/viewer.routes.test.ts b/packages/cli/src/__tests__/viewer.routes.test.ts new file mode 100644 index 0000000..5162406 --- /dev/null +++ b/packages/cli/src/__tests__/viewer.routes.test.ts @@ -0,0 +1,93 @@ +import fs from "node:fs/promises"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; +import { type Viewer, ViewerSchema } from "@stagereview/types/viewer"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { viewerRoutes } from "../routes/viewer.js"; +import { LOOPBACK_HOST, type ServerHandle, startServer } from "../server.js"; + +let tmpDir: string; +let webDist: string; +let binDir: string; +let originalPath: string | undefined; +const handles: ServerHandle[] = []; + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-cli-viewer-")); + webDist = path.join(tmpDir, "web-dist"); + binDir = path.join(tmpDir, "bin"); + await fs.mkdir(webDist); + await fs.writeFile(path.join(webDist, "index.html"), ""); + await fs.mkdir(binDir); + // Prepend a fake `gh` so the route never reaches the real CLI or the network. + originalPath = process.env.PATH; + process.env.PATH = `${binDir}${path.delimiter}${originalPath ?? ""}`; +}); + +afterEach(async () => { + while (handles.length > 0) { + const h = handles.pop(); + if (h) await h.close(); + } + process.env.PATH = originalPath; + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +async function writeFakeGh(body: string): Promise { + const file = path.join(binDir, "gh"); + await fs.writeFile(file, `#!/bin/sh\n${body}\n`); + await fs.chmod(file, 0o755); +} + +async function start(): Promise { + const handle = await startServer({ webDistPath: webDist, routes: viewerRoutes() }); + handles.push(handle); + return handle.port; +} + +function get(port: number, p: string): Promise<{ status: number; body: string }> { + return new Promise((resolve, reject) => { + const req = http.request( + { hostname: LOOPBACK_HOST, port, method: "GET", path: p, agent: false }, + (res) => { + const chunks: Buffer[] = []; + res.on("data", (c: Buffer) => chunks.push(c)); + res.on("end", () => + resolve({ status: res.statusCode ?? 0, body: Buffer.concat(chunks).toString("utf8") }), + ); + }, + ); + req.on("error", reject); + req.end(); + }); +} + +describe("viewer API", () => { + it("returns the authenticated GitHub user from gh", async () => { + await writeFakeGh( + `if [ "$1" = "api" ] && [ "$2" = "user" ]; then + echo '{"login":"octocat","name":"The Octocat","avatar_url":"https://avatars.example/oct.png"}' +else exit 1; fi`, + ); + const port = await start(); + + const res = await get(port, "/api/viewer"); + expect(res.status).toBe(200); + expect(JSON.parse(res.body)).toEqual({ + name: "The Octocat", + avatarUrl: "https://avatars.example/oct.png", + }); + }); + + it("falls back to a name with no avatar when gh is unavailable", async () => { + await writeFakeGh("exit 1"); + const port = await start(); + + const res = await get(port, "/api/viewer"); + expect(res.status).toBe(200); + const viewer: Viewer = ViewerSchema.parse(JSON.parse(res.body)); + expect(viewer.name.length).toBeGreaterThan(0); + expect(viewer.avatarUrl).toBeNull(); + }); +}); diff --git a/packages/cli/src/git.ts b/packages/cli/src/git.ts index 95fd013..cc13868 100644 --- a/packages/cli/src/git.ts +++ b/packages/cli/src/git.ts @@ -50,6 +50,19 @@ function readOriginUrl(repoRoot: string): string | null { } } +/** Configured `user.name` for the repo, or null when unset. Used as a viewer-identity fallback. */ +export function readGitUserName(repoRoot: string): string | null { + try { + const out = execFileSync("git", ["-C", repoRoot, "config", "user.name"], { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); + return out || null; + } catch { + return null; + } +} + export function buildDiffArgs(run: ChapterRunRow): string[] { if (run.scopeKind === SCOPE_KIND.COMMITTED) { return ["diff", "--no-color", `${run.baseSha}..${run.headSha}`]; diff --git a/packages/cli/src/github/index.ts b/packages/cli/src/github/index.ts index 8b056e2..c77aed6 100644 --- a/packages/cli/src/github/index.ts +++ b/packages/cli/src/github/index.ts @@ -17,3 +17,4 @@ export { resolvePullRequestRefs, } from "./pull-request-ref.js"; export { type GitHubRepo, isGitHubRemote, parseGitHubRepo } from "./repo.js"; +export { type GitHubViewer, getGitHubViewer } from "./viewer.js"; diff --git a/packages/cli/src/github/viewer.ts b/packages/cli/src/github/viewer.ts new file mode 100644 index 0000000..a92a76a --- /dev/null +++ b/packages/cli/src/github/viewer.ts @@ -0,0 +1,35 @@ +import { z } from "zod"; +import { gh } from "./exec.js"; + +const GhViewerSchema = z.object({ + login: z.string(), + name: z.string().nullable().optional(), + avatar_url: z.string().optional(), +}); + +export interface GitHubViewer { + login: string; + name: string | null; + avatarUrl: string; +} + +/** + * The authenticated GitHub user via `gh api user`, or null when `gh` is missing, + * unauthenticated, or offline. Never throws — the viewer is a display nicety, so + * callers fall back to a local identity. + */ +export async function getGitHubViewer(repoRoot: string): Promise { + try { + const stdout = await gh(["api", "user", "--jq", "{login,name,avatar_url}"], repoRoot); + const parsed = GhViewerSchema.safeParse(JSON.parse(stdout)); + if (!parsed.success) return null; + const { login, name, avatar_url } = parsed.data; + return { + login, + name: name ?? null, + avatarUrl: avatar_url || `https://github.com/${encodeURIComponent(login)}.png`, + }; + } catch { + return null; + } +} diff --git a/packages/cli/src/routes/viewer.ts b/packages/cli/src/routes/viewer.ts new file mode 100644 index 0000000..d19de57 --- /dev/null +++ b/packages/cli/src/routes/viewer.ts @@ -0,0 +1,28 @@ +import type { Viewer } from "@stagereview/types/viewer"; +import { readGitUserName, readRepoRoot } from "../git.js"; +import { getGitHubViewer } from "../github/index.js"; +import type { Route } from "../server.js"; +import { writeJson } from "./json.js"; + +export function viewerRoutes(): Route[] { + return [ + { + method: "GET", + pattern: "/api/viewer", + handler: async (_req, res) => { + writeJson(res, 200, await resolveViewer()); + }, + }, + ]; +} + +// gh-authenticated user → git config user.name → a generic local label. Each step +// degrades silently, so the byline always has something to render. +async function resolveViewer(): Promise { + const repoRoot = readRepoRoot(); + const ghViewer = await getGitHubViewer(repoRoot); + if (ghViewer) return { name: ghViewer.name ?? ghViewer.login, avatarUrl: ghViewer.avatarUrl }; + const gitName = readGitUserName(repoRoot); + if (gitName) return { name: gitName, avatarUrl: null }; + return { name: "You", avatarUrl: null }; +} diff --git a/packages/cli/src/show.ts b/packages/cli/src/show.ts index 40e1ef8..2d71487 100644 --- a/packages/cli/src/show.ts +++ b/packages/cli/src/show.ts @@ -12,6 +12,7 @@ import { pullRequestRoutes } from "./routes/pull-request.js"; import { pullRequestMutationRoutes } from "./routes/pull-request-mutations.js"; import { runRoutes } from "./routes/runs.js"; import { viewStateRoutes } from "./routes/view-state.js"; +import { viewerRoutes } from "./routes/viewer.js"; import { insertChaptersFile } from "./runs/import-chapters.js"; import { type AgentOutput, @@ -35,6 +36,7 @@ export async function show(jsonPath: string, options: DiffScopeOptions): Promise ...runRoutes(db), ...viewStateRoutes(db), ...commentRoutes(db), + ...viewerRoutes(), ...diffRoutes(db), ...pullRequestRoutes(db), ...pullRequestMutationRoutes(db), diff --git a/packages/types/package.json b/packages/types/package.json index 3781db0..f9635a9 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -12,7 +12,8 @@ "./parsed-diff": "./src/parsed-diff.ts", "./prologue": "./src/prologue.ts", "./pull-request": "./src/pull-request.ts", - "./view-state": "./src/view-state.ts" + "./view-state": "./src/view-state.ts", + "./viewer": "./src/viewer.ts" }, "files": [ "src" diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 4aa846d..18eb175 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -5,3 +5,4 @@ export * from "./parsed-diff.ts"; export * from "./prologue.ts"; export * from "./pull-request.ts"; export * from "./view-state.ts"; +export * from "./viewer.ts"; diff --git a/packages/types/src/viewer.ts b/packages/types/src/viewer.ts new file mode 100644 index 0000000..74fb550 --- /dev/null +++ b/packages/types/src/viewer.ts @@ -0,0 +1,9 @@ +import { z } from "zod"; + +// The local reviewer's display identity, resolved from `gh` (falling back to git +// config, then a generic label). `avatarUrl` is null when no GitHub avatar is known. +export const ViewerSchema = z.object({ + name: z.string(), + avatarUrl: z.string().nullable(), +}); +export type Viewer = z.infer; diff --git a/packages/web/src/components/comments/comment-thread.tsx b/packages/web/src/components/comments/comment-thread.tsx index 9c3df81..a416762 100644 --- a/packages/web/src/components/comments/comment-thread.tsx +++ b/packages/web/src/components/comments/comment-thread.tsx @@ -9,7 +9,7 @@ import { AlertDialogHeader, AlertDialogTitle, } from "@/components/ui/alert-dialog"; -import { Avatar, AvatarFallback } from "@/components/ui/avatar"; +import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; import { Button } from "@/components/ui/button"; import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@/components/ui/collapsible"; import { Markdown } from "@/components/ui/markdown"; @@ -17,6 +17,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip import { useCommentThreadsContext } from "@/lib/comment-threads-context"; import { formatTimeAgo } from "@/lib/format"; import type { Comment, CommentThread } from "@/lib/use-comment-threads"; +import { useViewer } from "@/lib/use-viewer"; import { cn } from "@/lib/utils"; import { CommentActions } from "./comment-actions"; import { CommentForm } from "./comment-form"; @@ -231,14 +232,16 @@ function ResolveButton({ isResolved, onToggle }: { isResolved: boolean; onToggle } function CommentByline({ createdAt }: { createdAt: string }) { + const viewer = useViewer(); return (

+ {viewer.avatarUrl && } - You + {viewer.name} diff --git a/packages/web/src/lib/use-viewer.ts b/packages/web/src/lib/use-viewer.ts new file mode 100644 index 0000000..163dc84 --- /dev/null +++ b/packages/web/src/lib/use-viewer.ts @@ -0,0 +1,24 @@ +import { type Viewer, ViewerSchema } from "@stagereview/types/viewer"; +import { useQuery } from "@tanstack/react-query"; +import { jsonFetch } from "./use-view-state"; + +const FALLBACK_VIEWER: Viewer = { name: "You", avatarUrl: null }; + +async function fetchViewer(): Promise { + const raw = await jsonFetch("/api/viewer"); + return ViewerSchema.parse(raw); +} + +/** + * The local reviewer's display identity (resolved server-side from gh/git). The + * identity is stable for the session, so it's cached indefinitely; while it loads + * or if it fails, callers get a generic "You". + */ +export function useViewer(): Viewer { + const { data } = useQuery({ + queryKey: ["viewer"], + queryFn: fetchViewer, + staleTime: Number.POSITIVE_INFINITY, + }); + return data ?? FALLBACK_VIEWER; +} From 34e2ec9cd2013ffdd611f725c79ee2b4fe2e09b5 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 22:47:01 -0700 Subject: [PATCH 16/30] fix(PRO-546): show the GitHub login in the comment byline, not the display name --- packages/cli/src/__tests__/viewer.routes.test.ts | 5 +++-- packages/cli/src/github/viewer.ts | 7 ++----- packages/cli/src/routes/viewer.ts | 3 ++- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/__tests__/viewer.routes.test.ts b/packages/cli/src/__tests__/viewer.routes.test.ts index 5162406..ee9e94c 100644 --- a/packages/cli/src/__tests__/viewer.routes.test.ts +++ b/packages/cli/src/__tests__/viewer.routes.test.ts @@ -64,7 +64,8 @@ function get(port: number, p: string): Promise<{ status: number; body: string }> } describe("viewer API", () => { - it("returns the authenticated GitHub user from gh", async () => { + it("shows the GitHub login (not the display name) from gh", async () => { + // gh returns both a login and a display name; the byline should use the login. await writeFakeGh( `if [ "$1" = "api" ] && [ "$2" = "user" ]; then echo '{"login":"octocat","name":"The Octocat","avatar_url":"https://avatars.example/oct.png"}' @@ -75,7 +76,7 @@ else exit 1; fi`, const res = await get(port, "/api/viewer"); expect(res.status).toBe(200); expect(JSON.parse(res.body)).toEqual({ - name: "The Octocat", + name: "octocat", avatarUrl: "https://avatars.example/oct.png", }); }); diff --git a/packages/cli/src/github/viewer.ts b/packages/cli/src/github/viewer.ts index a92a76a..1869040 100644 --- a/packages/cli/src/github/viewer.ts +++ b/packages/cli/src/github/viewer.ts @@ -3,13 +3,11 @@ import { gh } from "./exec.js"; const GhViewerSchema = z.object({ login: z.string(), - name: z.string().nullable().optional(), avatar_url: z.string().optional(), }); export interface GitHubViewer { login: string; - name: string | null; avatarUrl: string; } @@ -20,13 +18,12 @@ export interface GitHubViewer { */ export async function getGitHubViewer(repoRoot: string): Promise { try { - const stdout = await gh(["api", "user", "--jq", "{login,name,avatar_url}"], repoRoot); + const stdout = await gh(["api", "user", "--jq", "{login,avatar_url}"], repoRoot); const parsed = GhViewerSchema.safeParse(JSON.parse(stdout)); if (!parsed.success) return null; - const { login, name, avatar_url } = parsed.data; + const { login, avatar_url } = parsed.data; return { login, - name: name ?? null, avatarUrl: avatar_url || `https://github.com/${encodeURIComponent(login)}.png`, }; } catch { diff --git a/packages/cli/src/routes/viewer.ts b/packages/cli/src/routes/viewer.ts index d19de57..a7104fc 100644 --- a/packages/cli/src/routes/viewer.ts +++ b/packages/cli/src/routes/viewer.ts @@ -21,7 +21,8 @@ export function viewerRoutes(): Route[] { async function resolveViewer(): Promise { const repoRoot = readRepoRoot(); const ghViewer = await getGitHubViewer(repoRoot); - if (ghViewer) return { name: ghViewer.name ?? ghViewer.login, avatarUrl: ghViewer.avatarUrl }; + // Show the GitHub login (e.g. "dastratakos"), not the display name. + if (ghViewer) return { name: ghViewer.login, avatarUrl: ghViewer.avatarUrl }; const gitName = readGitUserName(repoRoot); if (gitName) return { name: gitName, avatarUrl: null }; return { name: "You", avatarUrl: null }; From b555001848df806fb7704876ca5a63f2806367b5 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sun, 7 Jun 2026 22:48:32 -0700 Subject: [PATCH 17/30] chore(PRO-546): drop a redundant comment in the viewer route --- packages/cli/src/routes/viewer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/routes/viewer.ts b/packages/cli/src/routes/viewer.ts index a7104fc..a656392 100644 --- a/packages/cli/src/routes/viewer.ts +++ b/packages/cli/src/routes/viewer.ts @@ -21,7 +21,6 @@ export function viewerRoutes(): Route[] { async function resolveViewer(): Promise { const repoRoot = readRepoRoot(); const ghViewer = await getGitHubViewer(repoRoot); - // Show the GitHub login (e.g. "dastratakos"), not the display name. if (ghViewer) return { name: ghViewer.login, avatarUrl: ghViewer.avatarUrl }; const gitName = readGitUserName(repoRoot); if (gitName) return { name: gitName, avatarUrl: null }; From 20ff3e74a8f9653613110745fda368c5ed645f4e Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 8 Jun 2026 07:23:50 -0700 Subject: [PATCH 18/30] fix(PRO-546): degrade the viewer route to a fallback if readRepoRoot throws --- packages/cli/src/routes/viewer.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/routes/viewer.ts b/packages/cli/src/routes/viewer.ts index a656392..37d113b 100644 --- a/packages/cli/src/routes/viewer.ts +++ b/packages/cli/src/routes/viewer.ts @@ -16,13 +16,20 @@ export function viewerRoutes(): Route[] { ]; } -// gh-authenticated user → git config user.name → a generic local label. Each step -// degrades silently, so the byline always has something to render. +const FALLBACK_VIEWER: Viewer = { name: "You", avatarUrl: null }; + +// gh-authenticated user → git config user.name → a generic local label. Every +// step degrades to the fallback, so the byline always has something to render. async function resolveViewer(): Promise { - const repoRoot = readRepoRoot(); + let repoRoot: string; + try { + repoRoot = readRepoRoot(); + } catch { + return FALLBACK_VIEWER; + } const ghViewer = await getGitHubViewer(repoRoot); if (ghViewer) return { name: ghViewer.login, avatarUrl: ghViewer.avatarUrl }; const gitName = readGitUserName(repoRoot); if (gitName) return { name: gitName, avatarUrl: null }; - return { name: "You", avatarUrl: null }; + return FALLBACK_VIEWER; } From 8889afc48b94f8a1221caccd1cf2fbb05769d19e Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 8 Jun 2026 07:23:51 -0700 Subject: [PATCH 19/30] fix(PRO-546): keep the viewer query cached for the session with gcTime --- packages/web/src/lib/use-viewer.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/web/src/lib/use-viewer.ts b/packages/web/src/lib/use-viewer.ts index 163dc84..096fbdf 100644 --- a/packages/web/src/lib/use-viewer.ts +++ b/packages/web/src/lib/use-viewer.ts @@ -18,7 +18,10 @@ export function useViewer(): Viewer { const { data } = useQuery({ queryKey: ["viewer"], queryFn: fetchViewer, + // Identity is stable for the session: never refetch, and never GC the entry + // (staleTime alone would still let it be collected once unobserved). staleTime: Number.POSITIVE_INFINITY, + gcTime: Number.POSITIVE_INFINITY, }); return data ?? FALLBACK_VIEWER; } From 7f64cf323ebf7f6a61fbea99f8340e89d1df5019 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 8 Jun 2026 11:13:40 -0700 Subject: [PATCH 20/30] fix(PRO-546): treat unified change-deletion rows as the deletion side --- .../lib/__tests__/use-text-selection.test.ts | 39 ++++++++++++++++++- packages/web/src/lib/use-text-selection.ts | 6 ++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/packages/web/src/lib/__tests__/use-text-selection.test.ts b/packages/web/src/lib/__tests__/use-text-selection.test.ts index 1d3b2a6..c5a0f2d 100644 --- a/packages/web/src/lib/__tests__/use-text-selection.test.ts +++ b/packages/web/src/lib/__tests__/use-text-selection.test.ts @@ -1,5 +1,10 @@ +// @vitest-environment happy-dom import { describe, expect, it } from "vitest"; -import { buildSelectedLineRange, normalizeSelectedLineRange } from "../use-text-selection"; +import { + buildSelectedLineRange, + getLineSide, + normalizeSelectedLineRange, +} from "../use-text-selection"; describe("buildSelectedLineRange", () => { it("orders endpoints ascending and keeps the side", () => { @@ -37,3 +42,35 @@ describe("normalizeSelectedLineRange", () => { ).toEqual({ start: 4, side: "deletions", end: 9, endSide: "additions" }); }); }); + +describe("getLineSide", () => { + function splitLine(sideAttr: "data-additions" | "data-deletions"): HTMLElement { + const wrapper = document.createElement("div"); + wrapper.setAttribute(sideAttr, ""); + const line = document.createElement("div"); + line.setAttribute("data-line", "5"); + wrapper.appendChild(line); + return line; + } + + function unifiedLine(lineType: string): HTMLElement { + const line = document.createElement("div"); + line.setAttribute("data-line", "5"); + line.setAttribute("data-line-type", lineType); + return line; + } + + it("reads the side from the split-view column ancestor", () => { + expect(getLineSide(splitLine("data-deletions"))).toBe("deletions"); + expect(getLineSide(splitLine("data-additions"))).toBe("additions"); + }); + + it("treats a unified change-deletion row as the deletion side", () => { + expect(getLineSide(unifiedLine("change-deletion"))).toBe("deletions"); + }); + + it("treats unified change-addition and context rows as the addition side", () => { + expect(getLineSide(unifiedLine("change-addition"))).toBe("additions"); + expect(getLineSide(unifiedLine("context"))).toBe("additions"); + }); +}); diff --git a/packages/web/src/lib/use-text-selection.ts b/packages/web/src/lib/use-text-selection.ts index 54be8ec..5845627 100644 --- a/packages/web/src/lib/use-text-selection.ts +++ b/packages/web/src/lib/use-text-selection.ts @@ -70,15 +70,17 @@ function findLineElement(node: Node): HTMLElement | null { * Determines a line element's diff side from its `data-additions`/`data-deletions` * ancestor, falling back to its `data-line-type` for unified/single-sided diffs. */ -function getLineSide(lineEl: HTMLElement): DiffSide { +export function getLineSide(lineEl: HTMLElement): DiffSide { let current: HTMLElement | null = lineEl; while (current) { if (current.hasAttribute("data-additions")) return DIFF_SIDE.ADDITIONS; if (current.hasAttribute("data-deletions")) return DIFF_SIDE.DELETIONS; current = current.parentElement; } + // Unified view has no side-column ancestor; Pierre marks deletion rows as + // "change-deletion" (see rendered-line-target.ts), so match that too. const lineType = lineEl.getAttribute("data-line-type"); - if (lineType === "deletion") return DIFF_SIDE.DELETIONS; + if (lineType === "deletion" || lineType === "change-deletion") return DIFF_SIDE.DELETIONS; return DIFF_SIDE.ADDITIONS; } From db4b45acd32aa69091d288cb2ccc21a03fd1866a Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 8 Jun 2026 11:28:36 -0700 Subject: [PATCH 21/30] fix(PRO-546): reject cross-side gutter drag selections A split-view gutter drag that crosses from one column to the other emits a SelectedLineRange whose side and endSide differ. handleLineSelected opened a draft using only norm.side, so the thread would cover unrelated old/new line numbers on a single side. Extract toSingleSideSelection, which mirrors buildSelectedLineRange's null-on-cross-side contract, and use it for the gutter path too. --- .../components/chapter/pierre-diff-viewer.tsx | 12 +++---- .../lib/__tests__/use-text-selection.test.ts | 33 +++++++++++++++++++ packages/web/src/lib/use-text-selection.ts | 18 ++++++++++ 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/packages/web/src/components/chapter/pierre-diff-viewer.tsx b/packages/web/src/components/chapter/pierre-diff-viewer.tsx index d46c88d..eaf779d 100644 --- a/packages/web/src/components/chapter/pierre-diff-viewer.tsx +++ b/packages/web/src/components/chapter/pierre-diff-viewer.tsx @@ -32,7 +32,7 @@ import { import { resolveSyntaxTheme } from "@/lib/syntax-themes"; import type { CommentThread } from "@/lib/use-comment-threads"; import { useDiffSettings } from "@/lib/use-diff-settings"; -import { normalizeSelectedLineRange, useTextSelection } from "@/lib/use-text-selection"; +import { toSingleSideSelection, useTextSelection } from "@/lib/use-text-selection"; import { LineHighlightOverlay } from "./hunk-highlight-overlay"; import { TextSelectionPopup } from "./text-selection-popup"; @@ -372,13 +372,11 @@ export function PierreDiffViewer({ // also changes `selectedLines` and would otherwise trigger this. const handleLineSelected = useCallback((range: SelectedLineRange | null) => { if (isHoveringRef.current || !range) return; - const norm = normalizeSelectedLineRange(range); + // A thread anchors to one side, so cross-side gutter drags are ignored. + const selection = toSingleSideSelection(range); + if (!selection) return; setDraftError(null); - setDraft({ - side: norm.side ?? DIFF_SIDE.ADDITIONS, - startLine: norm.start, - endLine: norm.end, - }); + setDraft(selection); }, []); const options = useMemo( diff --git a/packages/web/src/lib/__tests__/use-text-selection.test.ts b/packages/web/src/lib/__tests__/use-text-selection.test.ts index c5a0f2d..463c506 100644 --- a/packages/web/src/lib/__tests__/use-text-selection.test.ts +++ b/packages/web/src/lib/__tests__/use-text-selection.test.ts @@ -4,6 +4,7 @@ import { buildSelectedLineRange, getLineSide, normalizeSelectedLineRange, + toSingleSideSelection, } from "../use-text-selection"; describe("buildSelectedLineRange", () => { @@ -43,6 +44,38 @@ describe("normalizeSelectedLineRange", () => { }); }); +describe("toSingleSideSelection", () => { + it("returns single-side draft endpoints for a same-side range", () => { + expect( + toSingleSideSelection({ start: 4, side: "additions", end: 9, endSide: "additions" }), + ).toEqual({ side: "additions", startLine: 4, endLine: 9 }); + }); + + it("orders endpoints when dragging upward", () => { + expect( + toSingleSideSelection({ start: 9, side: "deletions", end: 4, endSide: "deletions" }), + ).toEqual({ side: "deletions", startLine: 4, endLine: 9 }); + }); + + it("returns null for a cross-side gutter drag", () => { + expect( + toSingleSideSelection({ start: 1, side: "deletions", end: 3, endSide: "additions" }), + ).toBeNull(); + // An upward drag whose swap still leaves the sides mismatched is also cross-side. + expect( + toSingleSideSelection({ start: 9, side: "additions", end: 4, endSide: "deletions" }), + ).toBeNull(); + }); + + it("defaults to the addition side when Pierre omits the side", () => { + expect(toSingleSideSelection({ start: 2, end: 4 })).toEqual({ + side: "additions", + startLine: 2, + endLine: 4, + }); + }); +}); + describe("getLineSide", () => { function splitLine(sideAttr: "data-additions" | "data-deletions"): HTMLElement { const wrapper = document.createElement("div"); diff --git a/packages/web/src/lib/use-text-selection.ts b/packages/web/src/lib/use-text-selection.ts index 5845627..b189f2f 100644 --- a/packages/web/src/lib/use-text-selection.ts +++ b/packages/web/src/lib/use-text-selection.ts @@ -51,6 +51,24 @@ export function normalizeSelectedLineRange(range: SelectedLineRange): SelectedLi }; } +/** + * Resolves a Pierre gutter selection to single-side draft endpoints, or null if + * it spans both diff sides. A comment thread anchors to one side, so cross-side + * gutter drags are rejected — the same invariant {@link buildSelectedLineRange} + * enforces for the text-selection path. + */ +export function toSingleSideSelection( + range: SelectedLineRange, +): { side: DiffSide; startLine: number; endLine: number } | null { + const norm = normalizeSelectedLineRange(range); + if (norm.side && norm.endSide && norm.side !== norm.endSide) return null; + return { + side: norm.side ?? DIFF_SIDE.ADDITIONS, + startLine: norm.start, + endLine: norm.end, + }; +} + /** * Finds the closest ancestor element (including the node itself) with a * `data-line` attribute. Does not cross shadow DOM boundaries. From debad2ab73bdd3bc594d123992e0a6ff070cfaf7 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 8 Jun 2026 11:39:20 -0700 Subject: [PATCH 22/30] fix(PRO-546): surface a failed comment-threads fetch as a toast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The threads query exposed error/isLoading, but no consumer read them, so a load failure rendered identically to having no comments — the diff still shows, the overlay is just silently empty. Toast the error from the provider (one query instance → one toast); React Query only sets error once its retries exhaust, so transient blips stay quiet. Add a regression test covering the error-toast and the no-toast-on-empty-success paths. --- .../comment-threads-context.test.tsx | 59 +++++++++++++++++++ .../web/src/lib/comment-threads-context.tsx | 14 ++++- 2 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 packages/web/src/lib/__tests__/comment-threads-context.test.tsx diff --git a/packages/web/src/lib/__tests__/comment-threads-context.test.tsx b/packages/web/src/lib/__tests__/comment-threads-context.test.tsx new file mode 100644 index 0000000..cc6b945 --- /dev/null +++ b/packages/web/src/lib/__tests__/comment-threads-context.test.tsx @@ -0,0 +1,59 @@ +// @vitest-environment happy-dom + +import { render, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { toast } from "@/components/ui/sonner"; +import { CommentThreadsProvider } from "../comment-threads-context"; +import { makeWrapper } from "./fixtures"; + +vi.mock("@/components/ui/sonner", () => ({ toast: { error: vi.fn() } })); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.clearAllMocks(); +}); + +function stubFetch(status: number, body: string): void { + vi.stubGlobal( + "fetch", + vi.fn( + async () => new Response(body, { status, headers: { "Content-Type": "application/json" } }), + ), + ); +} + +describe("CommentThreadsProvider", () => { + it("surfaces a failed threads fetch as a toast so it isn't mistaken for no comments", async () => { + stubFetch(500, "boom"); + const { Wrapper } = makeWrapper(); + + render( + + diff + , + { wrapper: Wrapper }, + ); + + await waitFor(() => + expect(vi.mocked(toast.error)).toHaveBeenCalledWith( + "Couldn't load comments", + expect.anything(), + ), + ); + }); + + it("does not toast when the fetch succeeds with no comments", async () => { + stubFetch(200, "[]"); + const { Wrapper } = makeWrapper(); + + render( + + diff + , + { wrapper: Wrapper }, + ); + + await waitFor(() => expect(vi.mocked(fetch)).toHaveBeenCalledTimes(1)); + expect(vi.mocked(toast.error)).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/web/src/lib/comment-threads-context.tsx b/packages/web/src/lib/comment-threads-context.tsx index 8100b24..c71f4b4 100644 --- a/packages/web/src/lib/comment-threads-context.tsx +++ b/packages/web/src/lib/comment-threads-context.tsx @@ -1,4 +1,5 @@ -import { createContext, type ReactNode, useContext } from "react"; +import { createContext, type ReactNode, useContext, useEffect } from "react"; +import { toast } from "@/components/ui/sonner"; import { type UseCommentThreadsResult, useCommentThreads } from "./use-comment-threads"; const CommentThreadsContext = createContext(null); @@ -15,6 +16,17 @@ export function CommentThreadsProvider({ children: ReactNode; }) { const value = useCommentThreads(runId); + + // A failed threads fetch is otherwise indistinguishable from "no comments" — + // the diff still renders, but the overlay is silently empty. Surface it as a + // toast (React Query only sets `error` once its retries are exhausted). + useEffect(() => { + if (!value.error) return; + toast.error("Couldn't load comments", { + description: value.error instanceof Error ? value.error.message : undefined, + }); + }, [value.error]); + return {children}; } From 6bfa6c78986e383fad333bbd3086205c247d92ce Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 8 Jun 2026 16:42:51 -0700 Subject: [PATCH 23/30] fix(PRO-546): harden diff text-selection edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review-flagged gaps in the comment text-selection path: - toSingleSideSelection fell straight through to additions when only endSide was set; fall back to endSide first (mirrors normalizeSelected- LineRange), so a side-less range anchors to the side actually known. - mouseup was bound on the diff container, so releasing a multi-line drag past the last visible row (outside the diff) never fired — the drag stuck open and no popup appeared. Capture mouseup on the document instead. - A unified-view triple-click on a replacement line extends into the addition row that shares the deletion's number; the endLine>startLine guard missed it, so the cross-side range was dropped. Detect the trailing row by element identity and clamp to the clicked side. Add toSingleSideSelection unit cases for the endSide fallback. --- .../lib/__tests__/use-text-selection.test.ts | 10 ++++- packages/web/src/lib/use-text-selection.ts | 44 +++++++++++++------ 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/packages/web/src/lib/__tests__/use-text-selection.test.ts b/packages/web/src/lib/__tests__/use-text-selection.test.ts index 463c506..5d297f6 100644 --- a/packages/web/src/lib/__tests__/use-text-selection.test.ts +++ b/packages/web/src/lib/__tests__/use-text-selection.test.ts @@ -67,7 +67,15 @@ describe("toSingleSideSelection", () => { ).toBeNull(); }); - it("defaults to the addition side when Pierre omits the side", () => { + it("falls back to endSide when only it is present, before defaulting to additions", () => { + expect(toSingleSideSelection({ start: 2, end: 4, endSide: "deletions" })).toEqual({ + side: "deletions", + startLine: 2, + endLine: 4, + }); + }); + + it("defaults to the addition side when Pierre omits both sides", () => { expect(toSingleSideSelection({ start: 2, end: 4 })).toEqual({ side: "additions", startLine: 2, diff --git a/packages/web/src/lib/use-text-selection.ts b/packages/web/src/lib/use-text-selection.ts index b189f2f..407121b 100644 --- a/packages/web/src/lib/use-text-selection.ts +++ b/packages/web/src/lib/use-text-selection.ts @@ -63,7 +63,8 @@ export function toSingleSideSelection( const norm = normalizeSelectedLineRange(range); if (norm.side && norm.endSide && norm.side !== norm.endSide) return null; return { - side: norm.side ?? DIFF_SIDE.ADDITIONS, + // Prefer the explicit side, then the only-known endSide, before defaulting. + side: norm.side ?? norm.endSide ?? DIFF_SIDE.ADDITIONS, startLine: norm.start, endLine: norm.end, }; @@ -208,18 +209,29 @@ export function useTextSelection(containerRef: React.RefObject startLine && range.endOffset === 0; + // A triple-click selects a full line, but the browser extends the range to + // offset 0 of the row *after* it, which shouldn't be included. Detect that + // trailing row by element identity, not line number: in unified view it can + // be the replacement line sharing the clicked line's number on the other side. + const isTripleClick = range.endOffset === 0 && endLineEl !== startLineEl; if (isTripleClick) { - endLine--; - if (endLine === startLine) { - endSide = startSide; + if (endLine > startLine) { + // Drop the trailing offset-0 row and re-resolve the now-last line's side. + endLine--; + if (endLine === startLine) { + endSide = startSide; + } else { + const sideContainer = + endLineEl.closest("[data-additions], [data-deletions]") ?? shadowRoot ?? container; + const adjustedEl = sideContainer.querySelector( + `[data-line="${endLine}"]`, + ); + if (adjustedEl) endSide = getLineSide(adjustedEl); + } } else { - const sideContainer = - endLineEl.closest("[data-additions], [data-deletions]") ?? shadowRoot ?? container; - const adjustedEl = sideContainer.querySelector(`[data-line="${endLine}"]`); - if (adjustedEl) endSide = getLineSide(adjustedEl); + // Trailing row shares the clicked line's number (unified replacement): + // clamp to the clicked line's side so the range stays single-sided. + endSide = startSide; } } @@ -265,7 +277,13 @@ export function useTextSelection(containerRef: React.RefObject Date: Mon, 8 Jun 2026 16:42:51 -0700 Subject: [PATCH 24/30] fix(PRO-546): dedupe the comment-load error toast Give the failed-fetch toast a stable id so a re-fire (StrictMode double- mount, remount with a cached error, or refetch failing with a new error reference) updates one toast instead of stacking duplicates. --- .../web/src/lib/__tests__/comment-threads-context.test.tsx | 2 +- packages/web/src/lib/comment-threads-context.tsx | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/web/src/lib/__tests__/comment-threads-context.test.tsx b/packages/web/src/lib/__tests__/comment-threads-context.test.tsx index cc6b945..8f54a09 100644 --- a/packages/web/src/lib/__tests__/comment-threads-context.test.tsx +++ b/packages/web/src/lib/__tests__/comment-threads-context.test.tsx @@ -37,7 +37,7 @@ describe("CommentThreadsProvider", () => { await waitFor(() => expect(vi.mocked(toast.error)).toHaveBeenCalledWith( "Couldn't load comments", - expect.anything(), + expect.objectContaining({ id: "comment-threads-error" }), ), ); }); diff --git a/packages/web/src/lib/comment-threads-context.tsx b/packages/web/src/lib/comment-threads-context.tsx index c71f4b4..217620f 100644 --- a/packages/web/src/lib/comment-threads-context.tsx +++ b/packages/web/src/lib/comment-threads-context.tsx @@ -22,7 +22,10 @@ export function CommentThreadsProvider({ // toast (React Query only sets `error` once its retries are exhausted). useEffect(() => { if (!value.error) return; + // Stable id so a re-fire (StrictMode double-mount, remount with a cached error, + // refetch failing with a new error reference) updates one toast instead of stacking. toast.error("Couldn't load comments", { + id: "comment-threads-error", description: value.error instanceof Error ? value.error.message : undefined, }); }, [value.error]); From 7117d9ff39795d4a1611285209b21c3c69fabc0d Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 8 Jun 2026 16:53:53 -0700 Subject: [PATCH 25/30] fix(PRO-546): don't let gutter actions discard an open comment draft The text-selection popup is suppressed while a composer is open (draft !== null), but the gutter '+' and gutter drag-select both replaced the draft anchor unconditionally. Since the composer's text lives in CommentForm's own state, moving the anchor unmounts the form and drops in-progress text. Guard both gutter paths on a draftRef (a render-synced mirror, so the Pierre callbacks keep stable identity and don't re-init the diff). --- .../src/components/chapter/pierre-diff-viewer.tsx | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/web/src/components/chapter/pierre-diff-viewer.tsx b/packages/web/src/components/chapter/pierre-diff-viewer.tsx index eaf779d..a07d674 100644 --- a/packages/web/src/components/chapter/pierre-diff-viewer.tsx +++ b/packages/web/src/components/chapter/pierre-diff-viewer.tsx @@ -226,6 +226,11 @@ export function PierreDiffViewer({ ); // The line range the user is composing a new comment on (null when idle). const [draft, setDraft] = useState(null); + // Mirror the draft into a ref so the Pierre callbacks below can check "is a + // composer already open?" without taking `draft` as a dep — that would change + // their identity and re-init the diff. Read in handlers only, never in render. + const draftRef = useRef(draft); + draftRef.current = draft; const [draftError, setDraftError] = useState(null); const { selectionInfo, clearSelection } = useTextSelection(diffContainerRef); @@ -337,6 +342,8 @@ export function PierreDiffViewer({ aria-label="Add comment" style={GUTTER_BUTTON_STYLE} onClick={() => { + // Don't replace an open composer's anchor — it would drop typed text. + if (draftRef.current !== null) return; const hovered = getHoveredLine(); if (!hovered) return; setDraftError(null); @@ -368,10 +375,12 @@ export function PierreDiffViewer({ ); // Dragging across the line-number gutter selects a range and opens the composer - // for the whole span. Suppressed while hovering a thread, since the highlight - // also changes `selectedLines` and would otherwise trigger this. + // for the whole span. const handleLineSelected = useCallback((range: SelectedLineRange | null) => { - if (isHoveringRef.current || !range) return; + // Bail while hovering a thread (its highlight also fires onLineSelected) or while + // a composer is open, so a stray drag can't discard in-progress text — the + // text-selection path is guarded the same way (`draft === null`). + if (isHoveringRef.current || !range || draftRef.current !== null) return; // A thread anchors to one side, so cross-side gutter drags are ignored. const selection = toSingleSideSelection(range); if (!selection) return; From 0218864a4bef3e1ee8c7a78c139587397d51145a Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 8 Jun 2026 19:20:28 -0700 Subject: [PATCH 26/30] feat(PRO-546): allow multiple comment composers open at once Replace the single moving draft with a collection of independent composers: opening a comment on another line (gutter +, gutter drag, or text selection) now adds a composer instead of relocating the existing one. Each composer submits, cancels, and shows errors independently. This supersedes the earlier single-draft guard (7117d9f): rather than block a second comment to avoid discarding the first, both stay open, so nothing is ever discarded. Implementation notes: - Draft state is a keyed collection; one composer per (side, endLine) row. - Composer text lives in a parent draftBodies ref keyed by anchor, so a form keeps its text if it remounts when another draft opens or closes. - Pierre keys its annotation rows by array index, so a row can be reused for a different anchor when a draft is added/removed; the composer carries a stable per-anchor key to force a clean remount (re-reading its own text) rather than inheriting another composer's in-progress state. - Extract the draft/annotation logic into lib/comment-drafts.ts with unit tests. Known trade-off: opening/closing a composer briefly flashes the other annotation rows (Pierre re-applies the annotation layer); accepted as-is for now. --- .../components/chapter/pierre-diff-viewer.tsx | 183 ++++++++---------- .../src/components/comments/comment-form.tsx | 8 +- .../src/lib/__tests__/comment-drafts.test.ts | 111 +++++++++++ packages/web/src/lib/comment-drafts.ts | 92 +++++++++ 4 files changed, 293 insertions(+), 101 deletions(-) create mode 100644 packages/web/src/lib/__tests__/comment-drafts.test.ts create mode 100644 packages/web/src/lib/comment-drafts.ts diff --git a/packages/web/src/components/chapter/pierre-diff-viewer.tsx b/packages/web/src/components/chapter/pierre-diff-viewer.tsx index a07d674..bf9ce49 100644 --- a/packages/web/src/components/chapter/pierre-diff-viewer.tsx +++ b/packages/web/src/components/chapter/pierre-diff-viewer.tsx @@ -20,12 +20,22 @@ import { } from "react"; import { CommentForm } from "@/components/comments/comment-form"; import { CommentThreadView } from "@/components/comments/comment-thread"; +import { + buildCommentAnnotations, + type CommentDraft, + clearDraftBody, + type DraftBodies, + type DraftState, + findDraftAt, + isSameAnchor, + readDraftBody, + writeDraftBody, +} from "@/lib/comment-drafts"; import { useCommentThreadsContext } from "@/lib/comment-threads-context"; import { type AnnotatedLineRef, COMMENT_SIDE, DIFF_SIDE, - type DiffSide, type LineRef, SIDE_TO_DIFF, } from "@/lib/diff-types"; @@ -99,41 +109,6 @@ export function getVisibleLineRange( }; } -interface CommentDraft { - side: DiffSide; - startLine: number; - endLine: number; -} - -// Groups threads (plus any in-progress draft) by their anchor line so Pierre -// renders one annotation row per (side, line) directly below the diff line. -function buildCommentAnnotations( - threads: CommentThread[], - draft: CommentDraft | null, -): DiffLineAnnotation[] { - const bySideLine = new Map>>(); - const ensure = (side: DiffSide, line: number): DiffLineAnnotation => { - let byLine = bySideLine.get(side); - if (!byLine) { - byLine = new Map(); - bySideLine.set(side, byLine); - } - let entry = byLine.get(line); - if (!entry) { - entry = { side, lineNumber: line, metadata: [] }; - byLine.set(line, entry); - } - return entry; - }; - for (const thread of threads) ensure(thread.side, thread.endLine).metadata.push(thread); - if (draft) ensure(draft.side, draft.endLine); - const out: DiffLineAnnotation[] = []; - for (const byLine of bySideLine.values()) { - for (const entry of byLine.values()) out.push(entry); - } - return out; -} - type PierreDiffViewerProps = { filePath?: string; selectedLines?: SelectedLineRange | null; @@ -224,14 +199,12 @@ export function PierreDiffViewer({ () => (filePath ? (comments.threadsByFile.get(filePath) ?? []) : []), [comments.threadsByFile, filePath], ); - // The line range the user is composing a new comment on (null when idle). - const [draft, setDraft] = useState(null); - // Mirror the draft into a ref so the Pierre callbacks below can check "is a - // composer already open?" without taking `draft` as a dep — that would change - // their identity and re-init the diff. Read in handlers only, never in render. - const draftRef = useRef(draft); - draftRef.current = draft; - const [draftError, setDraftError] = useState(null); + // In-progress comment composers, one per anchor row — several can be open at once. + const [drafts, setDrafts] = useState([]); + // Composer text indexed by anchor, kept in a ref so typing never rebuilds the + // annotation list and a composer's text survives the remount that opening or + // closing another draft can trigger. + const draftBodiesRef = useRef(new Map()); const { selectionInfo, clearSelection } = useTextSelection(diffContainerRef); // Hovering a thread highlights its anchored lines. Highlighting sets Pierre's @@ -241,19 +214,31 @@ export function PierreDiffViewer({ const isHoveringRef = useRef(false); const lineAnnotations = useMemo( - () => buildCommentAnnotations(fileThreads, draft), - [fileThreads, draft], + () => buildCommentAnnotations(fileThreads, drafts), + [fileThreads, drafts], ); - const cancelDraft = useCallback(() => { - setDraft(null); - setDraftError(null); + // Open a composer at an anchor. A row holds at most one composer, so a repeat open + // on the same (side, endLine) is a no-op rather than a duplicate. + const openDraft = useCallback((anchor: CommentDraft) => { + setDrafts((prev) => + findDraftAt(prev, anchor.side, anchor.endLine) ? prev : [...prev, { ...anchor, error: null }], + ); + }, []); + + const closeDraft = useCallback((draft: CommentDraft) => { + clearDraftBody(draftBodiesRef.current, draft.side, draft.endLine); + setDrafts((prev) => prev.filter((d) => !isSameAnchor(d, draft.side, draft.endLine))); }, []); const handleCreateComment = useCallback( - async (body: string) => { - if (!draft || !filePath) return; - setDraftError(null); + async (draft: CommentDraft, body: string) => { + if (!filePath) return; + const setError = (error: string | null) => + setDrafts((prev) => + prev.map((d) => (isSameAnchor(d, draft.side, draft.endLine) ? { ...d, error } : d)), + ); + setError(null); try { await createThread({ filePath, @@ -262,13 +247,13 @@ export function PierreDiffViewer({ endLine: draft.endLine, body, }); - setDraft(null); + closeDraft(draft); } catch (err) { - setDraftError(err instanceof Error ? err.message : "Failed to add comment"); + setError(err instanceof Error ? err.message : "Failed to add comment"); throw err; // keep the composer open with the body intact } }, - [draft, filePath, createThread], + [filePath, createThread, closeDraft], ); const handleThreadMouseEnter = useCallback((thread: CommentThread) => { @@ -289,9 +274,8 @@ export function PierreDiffViewer({ const renderAnnotation = useCallback( (annotation: DiffLineAnnotation): ReactNode => { const threads = annotation.metadata ?? []; - const showComposer = - draft !== null && annotation.side === draft.side && annotation.lineNumber === draft.endLine; - if (threads.length === 0 && !showComposer) return null; + const draft = findDraftAt(drafts, annotation.side, annotation.lineNumber); + if (threads.length === 0 && !draft) return null; return (

))} - {showComposer && ( + {draft && ( + // Pierre keys annotation rows by array index, so a row can be reused + // for a different anchor when a draft is added/removed. Key the composer + // by its anchor to force a clean remount (re-reading its own draft text) + // instead of inheriting another composer's in-progress state. + writeDraftBody(draftBodiesRef.current, draft.side, draft.endLine, body) + } + onSubmit={(body) => handleCreateComment(draft, body)} + onCancel={() => closeDraft(draft)} /> )}
); }, - [ - draft, - draftError, - handleCreateComment, - cancelDraft, - handleThreadMouseEnter, - handleThreadMouseLeave, - ], + [drafts, handleCreateComment, closeDraft, handleThreadMouseEnter, handleThreadMouseLeave], ); const renderGutterUtility = useCallback( @@ -342,12 +328,9 @@ export function PierreDiffViewer({ aria-label="Add comment" style={GUTTER_BUTTON_STYLE} onClick={() => { - // Don't replace an open composer's anchor — it would drop typed text. - if (draftRef.current !== null) return; const hovered = getHoveredLine(); if (!hovered) return; - setDraftError(null); - setDraft({ + openDraft({ side: hovered.side, startLine: hovered.lineNumber, endLine: hovered.lineNumber, @@ -358,35 +341,35 @@ export function PierreDiffViewer({
), - [], + [openDraft], ); const handleCommentFromSelection = useCallback( (range: SelectedLineRange) => { - setDraftError(null); - setDraft({ + openDraft({ side: range.side ?? DIFF_SIDE.ADDITIONS, startLine: range.start, endLine: range.end, }); clearSelection(); }, - [clearSelection], + [openDraft, clearSelection], ); - // Dragging across the line-number gutter selects a range and opens the composer - // for the whole span. - const handleLineSelected = useCallback((range: SelectedLineRange | null) => { - // Bail while hovering a thread (its highlight also fires onLineSelected) or while - // a composer is open, so a stray drag can't discard in-progress text — the - // text-selection path is guarded the same way (`draft === null`). - if (isHoveringRef.current || !range || draftRef.current !== null) return; - // A thread anchors to one side, so cross-side gutter drags are ignored. - const selection = toSingleSideSelection(range); - if (!selection) return; - setDraftError(null); - setDraft(selection); - }, []); + // Dragging across the line-number gutter selects a range and opens a composer for the + // whole span. Several composers can be open at once, so this adds one rather than + // replacing any already-open draft. + const handleLineSelected = useCallback( + (range: SelectedLineRange | null) => { + // Bail only while hovering a thread, whose highlight also fires onLineSelected. + if (isHoveringRef.current || !range) return; + // A thread anchors to one side, so cross-side gutter drags are ignored. + const selection = toSingleSideSelection(range); + if (!selection) return; + openDraft(selection); + }, + [openDraft], + ); const options = useMemo( () => ({ @@ -446,15 +429,15 @@ export function PierreDiffViewer({ /> ) : null; - // Only show the popup when text is selected and no composer is already open. - const textSelectionPopup = - selectionInfo && draft === null ? ( - - ) : null; + // Show the popup whenever text is selected — several composers can be open at once, + // so a text-selection comment is always available. + const textSelectionPopup = selectionInfo ? ( + + ) : null; if (fileDiff) { return ( diff --git a/packages/web/src/components/comments/comment-form.tsx b/packages/web/src/components/comments/comment-form.tsx index c970a1c..d431d0d 100644 --- a/packages/web/src/components/comments/comment-form.tsx +++ b/packages/web/src/components/comments/comment-form.tsx @@ -11,6 +11,8 @@ interface CommentFormProps { error?: string | null; /** Pre-fill the textarea when editing an existing comment. */ initialBody?: string; + /** Reports each edit so a parent can persist an in-progress draft across remounts. */ + onBodyChange?: (body: string) => void; autoFocus?: boolean; } @@ -21,6 +23,7 @@ export function CommentForm({ placeholder = "Leave a comment", error, initialBody, + onBodyChange, autoFocus = true, }: CommentFormProps) { const [body, setBody] = useState(initialBody ?? ""); @@ -69,7 +72,10 @@ export function CommentForm({
{ + setBody(value); + onBodyChange?.(value); + }} textareaRef={textareaRef} disabled={isSubmitting} placeholder={placeholder} diff --git a/packages/web/src/lib/__tests__/comment-drafts.test.ts b/packages/web/src/lib/__tests__/comment-drafts.test.ts new file mode 100644 index 0000000..fb8fc5f --- /dev/null +++ b/packages/web/src/lib/__tests__/comment-drafts.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from "vitest"; +import { + buildCommentAnnotations, + type CommentDraft, + clearDraftBody, + type DraftBodies, + type DraftState, + findDraftAt, + isSameAnchor, + readDraftBody, + writeDraftBody, +} from "../comment-drafts"; +import type { CommentThread } from "../use-comment-threads"; + +function makeThread( + over: Partial & Pick, +): CommentThread { + return { + id: `t-${over.side}-${over.endLine}`, + filePath: "a.ts", + startLine: over.endLine, + resolvedAt: null, + createdAt: "2026-06-08T00:00:00.000Z", + updatedAt: "2026-06-08T00:00:00.000Z", + comments: [], + ...over, + }; +} + +function draftState(side: CommentDraft["side"], startLine: number, endLine: number): DraftState { + return { side, startLine, endLine, error: null }; +} + +function rowFor( + annotations: ReturnType, + side: CommentDraft["side"], + lineNumber: number, +) { + return annotations.find((a) => a.side === side && a.lineNumber === lineNumber); +} + +describe("buildCommentAnnotations", () => { + it("returns no annotations for no threads and no drafts", () => { + expect(buildCommentAnnotations([], [])).toEqual([]); + }); + + it("groups multiple threads on the same row into one annotation", () => { + const annotations = buildCommentAnnotations( + [ + makeThread({ id: "t1", side: "additions", endLine: 5 }), + makeThread({ id: "t2", side: "additions", endLine: 5 }), + ], + [], + ); + expect(annotations).toHaveLength(1); + expect(rowFor(annotations, "additions", 5)?.metadata).toHaveLength(2); + }); + + it("creates a thread-less row per open draft", () => { + const annotations = buildCommentAnnotations( + [], + [draftState("additions", 5, 5), draftState("deletions", 8, 10)], + ); + expect(annotations).toHaveLength(2); + expect(rowFor(annotations, "additions", 5)?.metadata).toEqual([]); + expect(rowFor(annotations, "deletions", 10)?.metadata).toEqual([]); + }); + + it("shares one row when a draft and a thread anchor to the same (side, endLine)", () => { + const annotations = buildCommentAnnotations( + [makeThread({ side: "additions", endLine: 5 })], + [draftState("additions", 3, 5)], + ); + expect(annotations).toHaveLength(1); + expect(rowFor(annotations, "additions", 5)?.metadata).toHaveLength(1); + }); +}); + +describe("draft anchor helpers", () => { + const additionsDraft = draftState("additions", 5, 5); + const deletionsDraft = { ...draftState("deletions", 8, 10), error: "boom" as string | null }; + const drafts = [additionsDraft, deletionsDraft]; + + it("matches a draft by side and endLine only", () => { + expect(isSameAnchor(additionsDraft, "additions", 5)).toBe(true); + expect(isSameAnchor(additionsDraft, "deletions", 5)).toBe(false); + expect(isSameAnchor(deletionsDraft, "deletions", 10)).toBe(true); + }); + + it("finds the draft occupying a given row, or undefined", () => { + expect(findDraftAt(drafts, "deletions", 10)?.error).toBe("boom"); + expect(findDraftAt(drafts, "additions", 99)).toBeUndefined(); + }); +}); + +describe("draft body store", () => { + it("reads, writes, and clears text keyed by (side, endLine)", () => { + const bodies: DraftBodies = new Map(); + expect(readDraftBody(bodies, "additions", 5)).toBe(""); + + writeDraftBody(bodies, "additions", 5, "hello"); + writeDraftBody(bodies, "deletions", 5, "other side"); + expect(readDraftBody(bodies, "additions", 5)).toBe("hello"); + // Same line number on the other side is a distinct entry. + expect(readDraftBody(bodies, "deletions", 5)).toBe("other side"); + + clearDraftBody(bodies, "additions", 5); + expect(readDraftBody(bodies, "additions", 5)).toBe(""); + expect(readDraftBody(bodies, "deletions", 5)).toBe("other side"); + }); +}); diff --git a/packages/web/src/lib/comment-drafts.ts b/packages/web/src/lib/comment-drafts.ts new file mode 100644 index 0000000..5df0e68 --- /dev/null +++ b/packages/web/src/lib/comment-drafts.ts @@ -0,0 +1,92 @@ +import type { DiffLineAnnotation } from "@pierre/diffs"; +import type { DiffSide } from "@/lib/diff-types"; +import type { CommentThread } from "@/lib/use-comment-threads"; + +/** An in-progress comment the reviewer is composing, anchored to a line range. */ +export interface CommentDraft { + side: DiffSide; + startLine: number; + endLine: number; +} + +/** A draft plus the last submit error for its composer (null while clean). */ +export interface DraftState extends CommentDraft { + error: string | null; +} + +/** + * In-progress composer text indexed by anchor (side → endLine → text). Held in a + * ref rather than React state so typing never rebuilds the annotation list, and so + * a composer's text survives the remount that adding/removing another draft causes. + */ +export type DraftBodies = Map>; + +export function readDraftBody(bodies: DraftBodies, side: DiffSide, endLine: number): string { + return bodies.get(side)?.get(endLine) ?? ""; +} + +export function writeDraftBody( + bodies: DraftBodies, + side: DiffSide, + endLine: number, + text: string, +): void { + let byLine = bodies.get(side); + if (!byLine) { + byLine = new Map(); + bodies.set(side, byLine); + } + byLine.set(endLine, text); +} + +export function clearDraftBody(bodies: DraftBodies, side: DiffSide, endLine: number): void { + bodies.get(side)?.delete(endLine); +} + +/** A draft is identified by the annotation row it renders on: its `(side, endLine)`. */ +export function isSameAnchor(draft: CommentDraft, side: DiffSide, endLine: number): boolean { + return draft.side === side && draft.endLine === endLine; +} + +/** + * The composer renders on the annotation row for its `(side, endLine)`, so at most + * one draft can occupy a given row — used to dedupe repeated opens on the same line. + */ +export function findDraftAt( + drafts: readonly DraftState[], + side: DiffSide, + endLine: number, +): DraftState | undefined { + return drafts.find((draft) => isSameAnchor(draft, side, endLine)); +} + +/** + * Groups threads and open drafts into one annotation row per `(side, endLine)`, so + * Pierre renders a row (existing comments and/or a composer) directly below the line. + */ +export function buildCommentAnnotations( + threads: readonly CommentThread[], + drafts: readonly CommentDraft[], +): DiffLineAnnotation[] { + const bySideLine = new Map>>(); + const ensure = (side: DiffSide, line: number): DiffLineAnnotation => { + let byLine = bySideLine.get(side); + if (!byLine) { + byLine = new Map(); + bySideLine.set(side, byLine); + } + let entry = byLine.get(line); + if (!entry) { + entry = { side, lineNumber: line, metadata: [] }; + byLine.set(line, entry); + } + return entry; + }; + for (const thread of threads) ensure(thread.side, thread.endLine).metadata.push(thread); + for (const draft of drafts) ensure(draft.side, draft.endLine); + const out: DiffLineAnnotation[] = []; + for (const byLine of bySideLine.values()) { + for (const entry of byLine.values()) out.push(entry); + } + return out; +} From 7dbe2164675222ad36dd8d3fc90ba9c3f5093d59 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Mon, 8 Jun 2026 19:31:36 -0700 Subject: [PATCH 27/30] fix(PRO-546): update an open draft's startLine on re-drag openDraft treated an existing composer on the same (side, endLine) row as a no-op, so re-dragging a range that shares the end line but changes the start (e.g. 3-10 -> 7-10) left the draft's startLine stale and created the thread with the wrong span. Upsert instead: re-opening the row adopts the new startLine (and clears any stale submit error). Extracted to upsertDraft with unit tests. --- .../components/chapter/pierre-diff-viewer.tsx | 9 +++-- .../src/lib/__tests__/comment-drafts.test.ts | 33 +++++++++++++++++++ packages/web/src/lib/comment-drafts.ts | 17 ++++++++++ 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/packages/web/src/components/chapter/pierre-diff-viewer.tsx b/packages/web/src/components/chapter/pierre-diff-viewer.tsx index bf9ce49..091108a 100644 --- a/packages/web/src/components/chapter/pierre-diff-viewer.tsx +++ b/packages/web/src/components/chapter/pierre-diff-viewer.tsx @@ -29,6 +29,7 @@ import { findDraftAt, isSameAnchor, readDraftBody, + upsertDraft, writeDraftBody, } from "@/lib/comment-drafts"; import { useCommentThreadsContext } from "@/lib/comment-threads-context"; @@ -218,12 +219,10 @@ export function PierreDiffViewer({ [fileThreads, drafts], ); - // Open a composer at an anchor. A row holds at most one composer, so a repeat open - // on the same (side, endLine) is a no-op rather than a duplicate. + // Open a composer at an anchor. A row holds at most one composer, so re-opening the + // same (side, endLine) adopts the new range's startLine rather than duplicating it. const openDraft = useCallback((anchor: CommentDraft) => { - setDrafts((prev) => - findDraftAt(prev, anchor.side, anchor.endLine) ? prev : [...prev, { ...anchor, error: null }], - ); + setDrafts((prev) => upsertDraft(prev, anchor)); }, []); const closeDraft = useCallback((draft: CommentDraft) => { diff --git a/packages/web/src/lib/__tests__/comment-drafts.test.ts b/packages/web/src/lib/__tests__/comment-drafts.test.ts index fb8fc5f..f755893 100644 --- a/packages/web/src/lib/__tests__/comment-drafts.test.ts +++ b/packages/web/src/lib/__tests__/comment-drafts.test.ts @@ -8,6 +8,7 @@ import { findDraftAt, isSameAnchor, readDraftBody, + upsertDraft, writeDraftBody, } from "../comment-drafts"; import type { CommentThread } from "../use-comment-threads"; @@ -93,6 +94,38 @@ describe("draft anchor helpers", () => { }); }); +describe("upsertDraft", () => { + it("appends a new draft when no composer occupies the row", () => { + const result = upsertDraft([draftState("additions", 5, 5)], draftState("deletions", 8, 10)); + expect(result).toHaveLength(2); + expect(findDraftAt(result, "deletions", 10)?.startLine).toBe(8); + }); + + it("adopts the new startLine when re-opening the same (side, endLine) row", () => { + const existing = { ...draftState("additions", 3, 10), error: "boom" as string | null }; + const result = upsertDraft([existing], draftState("additions", 7, 10)); + expect(result).toHaveLength(1); + expect(result[0]?.startLine).toBe(7); + // A re-drag clears any stale submit error. + expect(result[0]?.error).toBeNull(); + }); + + it("leaves other open drafts untouched when updating one", () => { + const other = draftState("deletions", 1, 4); + const result = upsertDraft( + [other, draftState("additions", 3, 10)], + draftState("additions", 7, 10), + ); + expect(result).toContain(other); + expect(findDraftAt(result, "additions", 10)?.startLine).toBe(7); + }); + + it("opens a separate composer for a different endLine", () => { + const result = upsertDraft([draftState("additions", 3, 10)], draftState("additions", 3, 15)); + expect(result).toHaveLength(2); + }); +}); + describe("draft body store", () => { it("reads, writes, and clears text keyed by (side, endLine)", () => { const bodies: DraftBodies = new Map(); diff --git a/packages/web/src/lib/comment-drafts.ts b/packages/web/src/lib/comment-drafts.ts index 5df0e68..210672a 100644 --- a/packages/web/src/lib/comment-drafts.ts +++ b/packages/web/src/lib/comment-drafts.ts @@ -60,6 +60,23 @@ export function findDraftAt( return drafts.find((draft) => isSameAnchor(draft, side, endLine)); } +/** + * Opens a composer for the anchor. Since a row holds at most one composer, re-opening + * the same `(side, endLine)` doesn't duplicate it — instead it adopts the new range's + * `startLine` (a re-drag that shares the end line but widens/narrows the span) and + * clears any stale submit error. + */ +export function upsertDraft(drafts: readonly DraftState[], anchor: CommentDraft): DraftState[] { + if (!findDraftAt(drafts, anchor.side, anchor.endLine)) { + return [...drafts, { ...anchor, error: null }]; + } + return drafts.map((draft) => + isSameAnchor(draft, anchor.side, anchor.endLine) + ? { ...draft, startLine: anchor.startLine, error: null } + : draft, + ); +} + /** * Groups threads and open drafts into one annotation row per `(side, endLine)`, so * Pierre renders a row (existing comments and/or a composer) directly below the line. From 933ac40a6364a38979743d121efc0783c4d68c62 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sat, 13 Jun 2026 16:08:54 -0700 Subject: [PATCH 28/30] fix(PRO-546): dismiss the comment-load error toast on recovery The failed-fetch toast has a stable id but nothing cleared it when a later refetch (or navigating to another run) succeeded, so a stale "Couldn't load comments" could linger while comments were visible. Dismiss it by id once `error` clears. Add a regression test for the recover-and-dismiss path. --- .../comment-threads-context.test.tsx | 37 ++++++++++++++++++- .../web/src/lib/comment-threads-context.tsx | 12 ++++-- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/packages/web/src/lib/__tests__/comment-threads-context.test.tsx b/packages/web/src/lib/__tests__/comment-threads-context.test.tsx index 8f54a09..8deb9b0 100644 --- a/packages/web/src/lib/__tests__/comment-threads-context.test.tsx +++ b/packages/web/src/lib/__tests__/comment-threads-context.test.tsx @@ -1,12 +1,12 @@ // @vitest-environment happy-dom -import { render, waitFor } from "@testing-library/react"; +import { act, render, waitFor } from "@testing-library/react"; import { afterEach, describe, expect, it, vi } from "vitest"; import { toast } from "@/components/ui/sonner"; import { CommentThreadsProvider } from "../comment-threads-context"; import { makeWrapper } from "./fixtures"; -vi.mock("@/components/ui/sonner", () => ({ toast: { error: vi.fn() } })); +vi.mock("@/components/ui/sonner", () => ({ toast: { error: vi.fn(), dismiss: vi.fn() } })); afterEach(() => { vi.unstubAllGlobals(); @@ -56,4 +56,37 @@ describe("CommentThreadsProvider", () => { await waitFor(() => expect(vi.mocked(fetch)).toHaveBeenCalledTimes(1)); expect(vi.mocked(toast.error)).not.toHaveBeenCalled(); }); + + it("dismisses the error toast once a later fetch recovers", async () => { + let calls = 0; + vi.stubGlobal( + "fetch", + vi.fn(async () => { + calls += 1; + return calls === 1 + ? new Response("boom", { status: 500 }) + : new Response("[]", { status: 200, headers: { "Content-Type": "application/json" } }); + }), + ); + const { client, Wrapper } = makeWrapper(); + + render( + + diff + , + { wrapper: Wrapper }, + ); + + await waitFor(() => expect(vi.mocked(toast.error)).toHaveBeenCalled()); + // Ignore the no-op dismiss that runs before any error appears. + vi.mocked(toast.dismiss).mockClear(); + + await act(async () => { + await client.refetchQueries(); + }); + + await waitFor(() => + expect(vi.mocked(toast.dismiss)).toHaveBeenCalledWith("comment-threads-error"), + ); + }); }); diff --git a/packages/web/src/lib/comment-threads-context.tsx b/packages/web/src/lib/comment-threads-context.tsx index 217620f..42d2862 100644 --- a/packages/web/src/lib/comment-threads-context.tsx +++ b/packages/web/src/lib/comment-threads-context.tsx @@ -4,6 +4,8 @@ import { type UseCommentThreadsResult, useCommentThreads } from "./use-comment-t const CommentThreadsContext = createContext(null); +const LOAD_ERROR_TOAST_ID = "comment-threads-error"; + /** * Provides the run's comment threads + mutations to the diff tree without * prop-drilling through FileDiffList. Mounted once at the run layout. @@ -19,13 +21,17 @@ export function CommentThreadsProvider({ // A failed threads fetch is otherwise indistinguishable from "no comments" — // the diff still renders, but the overlay is silently empty. Surface it as a - // toast (React Query only sets `error` once its retries are exhausted). + // toast (React Query only sets `error` once its retries are exhausted), and + // dismiss it once a later fetch recovers so a stale message doesn't linger. useEffect(() => { - if (!value.error) return; + if (!value.error) { + toast.dismiss(LOAD_ERROR_TOAST_ID); + return; + } // Stable id so a re-fire (StrictMode double-mount, remount with a cached error, // refetch failing with a new error reference) updates one toast instead of stacking. toast.error("Couldn't load comments", { - id: "comment-threads-error", + id: LOAD_ERROR_TOAST_ID, description: value.error instanceof Error ? value.error.message : undefined, }); }, [value.error]); From 8e8d05430e3d051b08b6758fb7e54eaf117a56ea Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sat, 13 Jun 2026 16:23:10 -0700 Subject: [PATCH 29/30] fix(PRO-546): clamp triple-click selection to the previous rendered line A triple-click on the last row before a collapsed hunk gap extends the range to offset 0 of the first row after the gap (e.g. 50 -> 200). Decrementing that row anchored the draft at an unrendered line (199), so Pierre had no row for the composer and no comment box appeared. Resolve the previous rendered line from the DOM instead of assuming endLine - 1; this also subsumes the unified replacement-line case. Add previousRenderedLine with unit tests. --- .../lib/__tests__/use-text-selection.test.ts | 23 ++++++++++ packages/web/src/lib/use-text-selection.ts | 43 +++++++++++-------- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/packages/web/src/lib/__tests__/use-text-selection.test.ts b/packages/web/src/lib/__tests__/use-text-selection.test.ts index 5d297f6..ead89fa 100644 --- a/packages/web/src/lib/__tests__/use-text-selection.test.ts +++ b/packages/web/src/lib/__tests__/use-text-selection.test.ts @@ -4,6 +4,7 @@ import { buildSelectedLineRange, getLineSide, normalizeSelectedLineRange, + previousRenderedLine, toSingleSideSelection, } from "../use-text-selection"; @@ -115,3 +116,25 @@ describe("getLineSide", () => { expect(getLineSide(unifiedLine("context"))).toBe("additions"); }); }); + +describe("previousRenderedLine", () => { + function appendLine(scope: HTMLElement, line: number): HTMLElement { + const el = document.createElement("div"); + el.setAttribute("data-line", String(line)); + scope.appendChild(el); + return el; + } + + it("returns the DOM-previous rendered line across a collapsed gap, not lineNumber - 1", () => { + const scope = document.createElement("div"); + const before = appendLine(scope, 50); + const after = appendLine(scope, 200); // lines 51–199 are collapsed: no DOM rows + expect(previousRenderedLine(scope, after)).toBe(before); + }); + + it("returns null for the first rendered line", () => { + const scope = document.createElement("div"); + const first = appendLine(scope, 50); + expect(previousRenderedLine(scope, first)).toBeNull(); + }); +}); diff --git a/packages/web/src/lib/use-text-selection.ts b/packages/web/src/lib/use-text-selection.ts index 407121b..e29d053 100644 --- a/packages/web/src/lib/use-text-selection.ts +++ b/packages/web/src/lib/use-text-selection.ts @@ -114,6 +114,18 @@ function getLineNumber(el: HTMLElement): number { return Number(el.getAttribute("data-line")); } +/** + * The rendered line element immediately before `lineEl` within `scope`, in document + * order, or null if it is the first. Resolved from the DOM rather than `lineNumber - 1` + * so a selection that overshoots to the start of a trailing row clamps correctly even + * across a collapsed hunk gap, where the previous rendered line isn't `lineNumber - 1`. + */ +export function previousRenderedLine(scope: ParentNode, lineEl: HTMLElement): HTMLElement | null { + const lines = Array.from(scope.querySelectorAll("[data-line]")); + const index = lines.indexOf(lineEl); + return index > 0 ? (lines[index - 1] ?? null) : null; +} + /** * Detects native text selection inside a Pierre diff container and converts it to * a {@link TextSelectionInfo} (line range + bounding rect). Returns null when @@ -209,28 +221,21 @@ export function useTextSelection(containerRef: React.RefObject startLine) { - // Drop the trailing offset-0 row and re-resolve the now-last line's side. - endLine--; - if (endLine === startLine) { - endSide = startSide; - } else { - const sideContainer = - endLineEl.closest("[data-additions], [data-deletions]") ?? shadowRoot ?? container; - const adjustedEl = sideContainer.querySelector( - `[data-line="${endLine}"]`, - ); - if (adjustedEl) endSide = getLineSide(adjustedEl); - } + const scope = + endLineEl.closest("[data-additions], [data-deletions]") ?? shadowRoot ?? container; + const prev = previousRenderedLine(scope, endLineEl); + if (prev) { + endLine = getLineNumber(prev); + endSide = getLineSide(prev); } else { - // Trailing row shares the clicked line's number (unified replacement): - // clamp to the clicked line's side so the range stays single-sided. + endLine = startLine; endSide = startSide; } } From 21d2f663faafe3234e05e2646a7ffe0a56ae49b6 Mon Sep 17 00:00:00 2001 From: Dean Stratakos <29683763+dastratakos@users.noreply.github.com> Date: Sat, 13 Jun 2026 16:36:59 -0700 Subject: [PATCH 30/30] fix(PRO-546): gate reply actions while another thread form is active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reply's edit/delete menu was gated only by !isEditing for that reply, so while editing the root (or another reply) or composing a reply, other replies still showed actions — clicking Edit then dropped the in-progress form's unsaved text. Gate reply actions on the thread being idle, matching how the root comment's actions already behave. --- packages/web/src/components/comments/comment-thread.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/web/src/components/comments/comment-thread.tsx b/packages/web/src/components/comments/comment-thread.tsx index a416762..a3d4cc3 100644 --- a/packages/web/src/components/comments/comment-thread.tsx +++ b/packages/web/src/components/comments/comment-thread.tsx @@ -168,6 +168,7 @@ export function CommentThreadView({ thread }: { thread: CommentThread }) { { @@ -251,6 +252,7 @@ function CommentByline({ createdAt }: { createdAt: string }) { function ReplyItem({ reply, + idle, isEditing, error, onEdit, @@ -259,6 +261,7 @@ function ReplyItem({ onDelete, }: { reply: Comment; + idle: boolean; isEditing: boolean; error: string | null; onEdit: () => void; @@ -270,7 +273,9 @@ function ReplyItem({
- {!isEditing && } + {/* Only when the whole thread is idle, so opening this reply's editor can't + discard another in-progress edit or reply (matches the root comment). */} + {idle && }
{isEditing ? (