diff --git a/README.md b/README.md index 2c3489f..6d37971 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,9 @@ In your AI agent, run: /stage-chapters ``` -This organizes your local changes into reviewable chapters and opens a browser UI. Everything happens on your machine. +This organizes your local changes into reviewable chapters and prints a local review URL for you to open when ready. Everything happens on your machine. + +You can leave draft feedback comments on files and diff lines in the browser UI. The agent receives them only after you press **Submit feedback**. ### Options diff --git a/mise.toml b/mise.toml new file mode 100644 index 0000000..20fedfa --- /dev/null +++ b/mise.toml @@ -0,0 +1,2 @@ +[tools] +node = "25.9.0" diff --git a/packages/cli/drizzle/0005_mixed_rick_jones.sql b/packages/cli/drizzle/0005_mixed_rick_jones.sql new file mode 100644 index 0000000..9ba3922 --- /dev/null +++ b/packages/cli/drizzle/0005_mixed_rick_jones.sql @@ -0,0 +1,15 @@ +CREATE TABLE `feedback_comment` ( + `id` text PRIMARY KEY NOT NULL, + `createdAt` integer NOT NULL, + `updatedAt` integer NOT NULL, + `runId` text NOT NULL, + `target` text NOT NULL, + `body` text NOT NULL, + `status` text DEFAULT 'draft' NOT NULL, + `submittedAt` text, + `submissionId` text, + FOREIGN KEY (`runId`) REFERENCES `chapter_run`(`id`) ON UPDATE no action ON DELETE cascade +); +--> statement-breakpoint +CREATE INDEX `feedback_comment_run_id_idx` ON `feedback_comment` (`runId`);--> statement-breakpoint +CREATE INDEX `feedback_comment_submission_id_idx` ON `feedback_comment` (`submissionId`); \ No newline at end of file diff --git a/packages/cli/drizzle/meta/0005_snapshot.json b/packages/cli/drizzle/meta/0005_snapshot.json new file mode 100644 index 0000000..b88d3cb --- /dev/null +++ b/packages/cli/drizzle/meta/0005_snapshot.json @@ -0,0 +1,715 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "2d778809-e452-40d5-8ada-d2b8b6688a31", + "prevId": "df482c89-af64-45e8-9e36-5fe43b4e209f", + "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 + }, + "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": {} + }, + "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": {} + }, + "feedback_comment": { + "name": "feedback_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 + }, + "runId": { + "name": "runId", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "target": { + "name": "target", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "body": { + "name": "body", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'draft'" + }, + "submittedAt": { + "name": "submittedAt", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "submissionId": { + "name": "submissionId", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "feedback_comment_run_id_idx": { + "name": "feedback_comment_run_id_idx", + "columns": [ + "runId" + ], + "isUnique": false + }, + "feedback_comment_submission_id_idx": { + "name": "feedback_comment_submission_id_idx", + "columns": [ + "submissionId" + ], + "isUnique": false + } + }, + "foreignKeys": { + "feedback_comment_runId_chapter_run_id_fk": { + "name": "feedback_comment_runId_chapter_run_id_fk", + "tableFrom": "feedback_comment", + "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 ff6c18c..9c28627 100644 --- a/packages/cli/drizzle/meta/_journal.json +++ b/packages/cli/drizzle/meta/_journal.json @@ -36,6 +36,13 @@ "when": 1777914813035, "tag": "0004_silly_firebird", "breakpoints": true + }, + { + "idx": 5, + "version": "6", + "when": 1780656551282, + "tag": "0005_mixed_rick_jones", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/cli/package.json b/packages/cli/package.json index 903cce8..1b9ef9a 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -50,7 +50,6 @@ "commander": "^14.0.3", "drizzle-orm": "^0.45.2", "ignore": "^7.0.5", - "open": "^11.0.0", "parse-diff": "^0.11.1", "zod": "^4.3.6" }, diff --git a/packages/cli/src/__tests__/feedback-sink.test.ts b/packages/cli/src/__tests__/feedback-sink.test.ts new file mode 100644 index 0000000..d15b6a8 --- /dev/null +++ b/packages/cli/src/__tests__/feedback-sink.test.ts @@ -0,0 +1,50 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import type { FeedbackSubmission } from "@stagereview/types/feedback"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { createFeedbackSink } from "../feedback-sink.js"; + +let tmpDir: string; + +beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "stage-cli-feedback-sink-")); +}); + +afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); +}); + +describe("feedback sink", () => { + it("prints submitted feedback and appends it to the run JSONL file", async () => { + const writes: string[] = []; + const sink = createFeedbackSink("/repo/root", "run-1", { + repoDataDir: tmpDir, + output: { write: (chunk) => writes.push(chunk) }, + }); + const submission: FeedbackSubmission = { + id: "submission-1", + runId: "run-1", + submittedAt: "2026-06-05T10:00:00.000Z", + comments: [ + { + id: "comment-1", + runId: "run-1", + target: { type: "file", filePath: "src/foo.ts" }, + body: "Please rename this.", + status: "submitted", + createdAt: "2026-06-05T09:59:00.000Z", + updatedAt: "2026-06-05T10:00:00.000Z", + submittedAt: "2026-06-05T10:00:00.000Z", + submissionId: "submission-1", + }, + ], + }; + + await sink.deliver(submission); + + const payload = JSON.stringify(submission); + expect(writes).toEqual([`STAGE_FEEDBACK_SUBMITTED ${payload}\n`]); + await expect(fs.readFile(sink.feedbackFilePath, "utf8")).resolves.toBe(`${payload}\n`); + }); +}); diff --git a/packages/cli/src/__tests__/feedback.routes.test.ts b/packages/cli/src/__tests__/feedback.routes.test.ts new file mode 100644 index 0000000..f732298 --- /dev/null +++ b/packages/cli/src/__tests__/feedback.routes.test.ts @@ -0,0 +1,225 @@ +import fs from "node:fs/promises"; +import http from "node:http"; +import os from "node:os"; +import path from "node:path"; +import { + FeedbackCommentResponseSchema, + FeedbackCommentsResponseSchema, + type FeedbackSubmission, + FeedbackSubmissionResponseSchema, +} from "@stagereview/types/feedback"; +import { eq } from "drizzle-orm"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { closeDb, getDb } from "../db/client.js"; +import { chapter, feedbackComment } from "../db/schema/index.js"; +import { feedbackRoutes } from "../routes/feedback.js"; +import { insertChaptersFile } from "../runs/import-chapters.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-feedback-")); + 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(deliveries: FeedbackSubmission[] = []): Promise { + const db = getDb({ dbPath }); + const handle = await startServer({ + webDistPath: webDist, + routes: [ + ...feedbackRoutes(db, { + deliverSubmission: async (submission) => { + deliveries.push(submission); + }, + }), + ], + }); + handles.push(handle); + return handle; +} + +interface JsonResponse { + status: number; + body: unknown; +} + +function request( + 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: + payload.length > 0 + ? { + "Content-Type": "application/json", + "Content-Length": Buffer.byteLength(payload).toString(), + } + : undefined, + }, + (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(): { runId: string; chapterId: string } { + const db = getDb({ dbPath }); + const result = insertChaptersFile(db, makeFixture(), makeRepoContext()); + const [chapterRow] = db + .select() + .from(chapter) + .where(eq(chapter.runId, result.runId)) + .limit(1) + .all(); + if (!chapterRow) throw new Error("seed: missing chapter"); + return { runId: chapterRow.runId, chapterId: chapterRow.id }; +} + +describe("feedback API", () => { + it("creates draft file and line feedback comments and returns them oldest-first", async () => { + const { runId, chapterId } = seedRun(); + const { port } = await startWithRoutes(); + + const file = await request(port, "POST", `/api/runs/${runId}/feedback`, { + target: { type: "file", filePath: "src/foo.ts", chapterId }, + body: "This file needs a clearer name.", + }); + expect(file.status).toBe(201); + + const line = await request(port, "POST", `/api/runs/${runId}/feedback`, { + target: { + type: "line", + filePath: "src/foo.ts", + chapterId, + range: { side: "additions", startLine: 2, endLine: 3 }, + }, + body: "Please simplify this branch.", + }); + expect(line.status).toBe(201); + + const res = await request(port, "GET", `/api/runs/${runId}/feedback`); + expect(res.status).toBe(200); + const body = FeedbackCommentsResponseSchema.parse(res.body); + expect(body.comments.map((comment) => comment.body)).toEqual([ + "This file needs a clearer name.", + "Please simplify this branch.", + ]); + expect(body.comments.map((comment) => comment.status)).toEqual(["draft", "draft"]); + expect(body.comments.map((comment) => comment.target.type)).toEqual(["file", "line"]); + }); + + it("edits and deletes only draft feedback", async () => { + const { runId, chapterId } = seedRun(); + const { port } = await startWithRoutes(); + + const created = await request(port, "POST", `/api/runs/${runId}/feedback`, { + target: { type: "file", filePath: "src/foo.ts", chapterId }, + body: "Initial note.", + }); + const createdBody = FeedbackCommentResponseSchema.parse(created.body); + const commentId = createdBody.comment.id; + + const edited = await request(port, "PATCH", `/api/runs/${runId}/feedback/${commentId}`, { + body: "Edited note.", + }); + expect(edited.status).toBe(200); + expect(FeedbackCommentResponseSchema.parse(edited.body).comment.body).toBe("Edited note."); + + const deleted = await request(port, "DELETE", `/api/runs/${runId}/feedback/${commentId}`); + expect(deleted.status).toBe(200); + + const res = await request(port, "GET", `/api/runs/${runId}/feedback`); + expect(FeedbackCommentsResponseSchema.parse(res.body).comments).toHaveLength(0); + }); + + it("submits all drafts as one delivered immutable batch", async () => { + const { runId, chapterId } = seedRun(); + const deliveries: FeedbackSubmission[] = []; + const { port } = await startWithRoutes(deliveries); + + await request(port, "POST", `/api/runs/${runId}/feedback`, { + target: { type: "file", filePath: "src/foo.ts", chapterId }, + body: "First note.", + }); + const second = await request(port, "POST", `/api/runs/${runId}/feedback`, { + target: { type: "file", filePath: "src/bar.ts" }, + body: "Second note.", + }); + const secondId = FeedbackCommentResponseSchema.parse(second.body).comment.id; + + const submitted = await request(port, "POST", `/api/runs/${runId}/feedback/submit`); + expect(submitted.status).toBe(200); + const submission = FeedbackSubmissionResponseSchema.parse(submitted.body).submission; + expect(submission.comments.map((comment) => comment.body)).toEqual([ + "First note.", + "Second note.", + ]); + expect(submission.comments.every((comment) => comment.status === "submitted")).toBe(true); + expect(deliveries).toEqual([submission]); + + const db = getDb({ dbPath }); + const rows = db.select().from(feedbackComment).where(eq(feedbackComment.runId, runId)).all(); + expect(rows.map((row) => row.submissionId)).toEqual([submission.id, submission.id]); + + const edit = await request(port, "PATCH", `/api/runs/${runId}/feedback/${secondId}`, { + body: "Should not edit.", + }); + expect(edit.status).toBe(409); + + const del = await request(port, "DELETE", `/api/runs/${runId}/feedback/${secondId}`); + expect(del.status).toBe(409); + + const emptySubmit = await request(port, "POST", `/api/runs/${runId}/feedback/submit`); + expect(emptySubmit.status).toBe(400); + }); + + it("rejects comments targeting a chapter from another run", async () => { + const first = seedRun(); + const second = seedRun(); + const { port } = await startWithRoutes(); + + const res = await request(port, "POST", `/api/runs/${first.runId}/feedback`, { + target: { type: "file", filePath: "src/foo.ts", chapterId: second.chapterId }, + body: "Wrong run.", + }); + + expect(res.status).toBe(400); + }); +}); diff --git a/packages/cli/src/db/path.ts b/packages/cli/src/db/path.ts index abbcc0b..3d1f19e 100644 --- a/packages/cli/src/db/path.ts +++ b/packages/cli/src/db/path.ts @@ -9,11 +9,11 @@ const DB_FILE = "db.sqlite"; const REPO_HASH_LEN = 12; export function getDbPath(): string { - const dir = ensureRepoDir(readRepoRoot()); + const dir = getRepoDataDir(); return path.join(dir, DB_FILE); } -function ensureRepoDir(repoRoot: string): string { +export function getRepoDataDir(repoRoot = readRepoRoot()): string { const hash = createHash("sha256").update(repoRoot.trim()).digest("hex").slice(0, REPO_HASH_LEN); const dir = path.join(homedir(), STAGE_HOME, hash); mkdirSync(dir, { recursive: true }); diff --git a/packages/cli/src/db/schema/feedback-comment.ts b/packages/cli/src/db/schema/feedback-comment.ts new file mode 100644 index 0000000..3856cce --- /dev/null +++ b/packages/cli/src/db/schema/feedback-comment.ts @@ -0,0 +1,31 @@ +import type { FeedbackCommentTarget } from "@stagereview/types/feedback"; +import { FEEDBACK_COMMENT_STATUS } from "@stagereview/types/feedback"; +import { index, sqliteTable, text } from "drizzle-orm/sqlite-core"; +import { chapterRun } from "./chapter-run.js"; +import { baseColumns } from "./columns.js"; + +export const feedbackComment = sqliteTable( + "feedback_comment", + { + ...baseColumns(), + runId: text() + .notNull() + .references(() => chapterRun.id, { onDelete: "cascade" }), + target: text({ mode: "json" }).$type().notNull(), + body: text().notNull(), + status: text({ + enum: [FEEDBACK_COMMENT_STATUS.DRAFT, FEEDBACK_COMMENT_STATUS.SUBMITTED], + }) + .notNull() + .default(FEEDBACK_COMMENT_STATUS.DRAFT), + submittedAt: text(), + submissionId: text(), + }, + (table) => [ + index("feedback_comment_run_id_idx").on(table.runId), + index("feedback_comment_submission_id_idx").on(table.submissionId), + ], +); + +export type FeedbackCommentRow = typeof feedbackComment.$inferSelect; +export type FeedbackCommentInsert = typeof feedbackComment.$inferInsert; diff --git a/packages/cli/src/db/schema/index.ts b/packages/cli/src/db/schema/index.ts index 8872e14..3daeacd 100644 --- a/packages/cli/src/db/schema/index.ts +++ b/packages/cli/src/db/schema/index.ts @@ -2,6 +2,7 @@ export * from "./chapter.js"; export * from "./chapter-file-view.js"; export * from "./chapter-run.js"; export * from "./chapter-view.js"; +export * from "./feedback-comment.js"; export * from "./file-view.js"; export * from "./key-change.js"; export * from "./key-change-view.js"; diff --git a/packages/cli/src/feedback-sink.ts b/packages/cli/src/feedback-sink.ts new file mode 100644 index 0000000..413794e --- /dev/null +++ b/packages/cli/src/feedback-sink.ts @@ -0,0 +1,39 @@ +import { mkdirSync } from "node:fs"; +import fs from "node:fs/promises"; +import path from "node:path"; +import type { FeedbackSubmission } from "@stagereview/types/feedback"; +import { getRepoDataDir } from "./db/path.js"; + +export interface FeedbackSink { + feedbackFilePath: string; + deliver: (submission: FeedbackSubmission) => Promise; +} + +interface WritableOutput { + write: (chunk: string) => unknown; +} + +interface FeedbackSinkOptions { + output?: WritableOutput; + repoDataDir?: string; +} + +export function createFeedbackSink( + repoRoot: string, + runId: string, + opts: FeedbackSinkOptions = {}, +): FeedbackSink { + const feedbackDir = path.join(opts.repoDataDir ?? getRepoDataDir(repoRoot), "feedback"); + mkdirSync(feedbackDir, { recursive: true }); + const feedbackFilePath = path.join(feedbackDir, `${runId}.jsonl`); + const output = opts.output ?? process.stdout; + + return { + feedbackFilePath, + deliver: async (submission) => { + const payload = JSON.stringify(submission); + output.write(`STAGE_FEEDBACK_SUBMITTED ${payload}\n`); + await fs.appendFile(feedbackFilePath, `${payload}\n`, "utf8"); + }, + }; +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 3518dba..570a016 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -50,7 +50,7 @@ program program .command("show") - .description("Load a chapters.json file and open it in a local browser") + .description("Load a chapters.json file and serve it at a local review URL") .argument("", "Path to a chapters.json file") .argument("[refs...]", "Git refs to diff, for example: main, main feature, or main..feature") .option("--base ", "Base ref to diff against (default: auto-detect main/master)") diff --git a/packages/cli/src/routes/diff.ts b/packages/cli/src/routes/diff.ts index 5a5e053..0cda31d 100644 --- a/packages/cli/src/routes/diff.ts +++ b/packages/cli/src/routes/diff.ts @@ -115,8 +115,9 @@ export function diffRoutes(db: StageDb): Route[] { ]; } -const MINUS_RE = /^--- (?:a\/)?(.+)$/m; -const PLUS_RE = /^\+\+\+ (?:b\/)?(.+)$/m; +const MINUS_RE = /^--- (.+)$/m; +const PLUS_RE = /^\+\+\+ (.+)$/m; +const DIFF_GIT_RE = /^diff --git (.+) (.+)$/m; const BINARY_RE = /^Binary files/m; interface ParsedFilePaths { @@ -138,12 +139,21 @@ function parseFilePathsFromPatch(patch: string): ParsedFilePaths[] { if (!text.startsWith("diff --git ")) continue; const isBinary = BINARY_RE.test(text); + const diffGit = text.match(DIFF_GIT_RE); + const headerOldPath = diffGit?.[1] ?? null; + const headerNewPath = diffGit?.[2] ?? null; const minus = text.match(MINUS_RE); const plus = text.match(PLUS_RE); - const oldPath = minus?.[1] && minus[1] !== "/dev/null" ? minus[1] : null; - const newPath = plus?.[1] && plus[1] !== "/dev/null" ? plus[1] : null; + const oldPath = + minus?.[1] && minus[1] !== "/dev/null" + ? normalizePatchPath(minus[1], headerOldPath, headerNewPath) + : null; + const newPath = + plus?.[1] && plus[1] !== "/dev/null" + ? normalizePatchPath(plus[1], headerNewPath, headerOldPath) + : null; results.push({ oldPath, newPath, isBinary }); } @@ -151,6 +161,32 @@ function parseFilePathsFromPatch(patch: string): ParsedFilePaths[] { return results; } +interface PrefixedPath { + prefix: string; + path: string; +} + +function splitDiffPrefix(value: string): PrefixedPath | null { + const slash = value.indexOf("/"); + if (slash <= 0) return null; + const prefix = value.slice(0, slash); + if (!/^(a|b|c|i|w|o|[12]|u)$/.test(prefix)) return null; + return { prefix, path: value.slice(slash + 1) }; +} + +function normalizePatchPath( + value: string, + matchingHeaderPath: string | null, + otherHeaderPath: string | null, +): string { + if (value !== matchingHeaderPath) return value; + const matching = splitDiffPrefix(matchingHeaderPath); + const other = otherHeaderPath ? splitDiffPrefix(otherHeaderPath) : null; + if (!matching || !other) return value; + if (matching.prefix === other.prefix) return value; + return matching.path; +} + async function getGitFileContent( cwd: string, ref: string, diff --git a/packages/cli/src/routes/feedback.ts b/packages/cli/src/routes/feedback.ts new file mode 100644 index 0000000..3bb75df --- /dev/null +++ b/packages/cli/src/routes/feedback.ts @@ -0,0 +1,290 @@ +import { randomUUID } from "node:crypto"; +import { + type CreateFeedbackCommentBody, + CreateFeedbackCommentBodySchema, + FEEDBACK_COMMENT_STATUS, + type FeedbackComment, + type FeedbackSubmission, + type UpdateFeedbackCommentBody, + UpdateFeedbackCommentBodySchema, +} from "@stagereview/types/feedback"; +import { and, asc, eq, inArray } from "drizzle-orm"; +import type { StageDb } from "../db/client.js"; +import { chapter, chapterRun, feedbackComment } from "../db/schema/index.js"; +import type { Route } from "../server.js"; +import { readJsonBody, writeJson } from "./json.js"; + +interface FeedbackRoutesOptions { + deliverSubmission?: (submission: FeedbackSubmission) => Promise; + onSubmitted?: () => void; +} + +type FeedbackCommentRow = typeof feedbackComment.$inferSelect; + +export function feedbackRoutes(db: StageDb, opts: FeedbackRoutesOptions = {}): Route[] { + return [ + { + method: "GET", + pattern: "/api/runs/:runId/feedback", + handler: (_req, res, params) => { + const runId = requireRunId(res, params.runId); + if (!runId) return; + if (!runExists(db, runId)) { + writeJson(res, 404, { error: `Run ${runId} not found` }); + return; + } + + const comments = db + .select() + .from(feedbackComment) + .where(eq(feedbackComment.runId, runId)) + .orderBy(asc(feedbackComment.createdAt)) + .all() + .map(mapFeedbackComment); + writeJson(res, 200, { comments }); + }, + }, + { + method: "POST", + pattern: "/api/runs/:runId/feedback", + handler: async (req, res, params) => { + const runId = requireRunId(res, params.runId); + if (!runId) return; + if (!runExists(db, runId)) { + writeJson(res, 404, { error: `Run ${runId} not found` }); + return; + } + + const parsed = await parseBody( + req, + res, + CreateFeedbackCommentBodySchema, + "Invalid feedback comment body", + ); + if (!parsed) return; + if (!targetBelongsToRun(db, runId, parsed.target.chapterId)) { + writeJson(res, 400, { error: "Feedback target chapter does not belong to this run" }); + return; + } + + const [row] = db + .insert(feedbackComment) + .values({ + runId, + target: parsed.target, + body: parsed.body, + status: FEEDBACK_COMMENT_STATUS.DRAFT, + }) + .returning() + .all(); + if (!row) throw new Error("feedback_comment insert returned no row"); + writeJson(res, 201, { comment: mapFeedbackComment(row) }); + }, + }, + { + method: "PATCH", + pattern: "/api/runs/:runId/feedback/:commentId", + handler: async (req, res, params) => { + const resolved = requireRunAndCommentId(res, params.runId, params.commentId); + if (!resolved) return; + const existing = findComment(db, resolved.runId, resolved.commentId); + if (!existing) { + writeJson(res, 404, { error: `Feedback comment ${resolved.commentId} not found` }); + return; + } + if (existing.status !== FEEDBACK_COMMENT_STATUS.DRAFT) { + writeJson(res, 409, { error: "Submitted feedback comments cannot be edited" }); + return; + } + + const parsed = await parseBody( + req, + res, + UpdateFeedbackCommentBodySchema, + "Invalid feedback update body", + ); + if (!parsed) return; + + const [row] = db + .update(feedbackComment) + .set({ body: parsed.body }) + .where(eq(feedbackComment.id, resolved.commentId)) + .returning() + .all(); + if (!row) throw new Error("feedback_comment update returned no row"); + writeJson(res, 200, { comment: mapFeedbackComment(row) }); + }, + }, + { + method: "DELETE", + pattern: "/api/runs/:runId/feedback/:commentId", + handler: (_req, res, params) => { + const resolved = requireRunAndCommentId(res, params.runId, params.commentId); + if (!resolved) return; + const existing = findComment(db, resolved.runId, resolved.commentId); + if (!existing) { + writeJson(res, 200, {}); + return; + } + if (existing.status !== FEEDBACK_COMMENT_STATUS.DRAFT) { + writeJson(res, 409, { error: "Submitted feedback comments cannot be deleted" }); + return; + } + db.delete(feedbackComment).where(eq(feedbackComment.id, resolved.commentId)).run(); + writeJson(res, 200, {}); + }, + }, + { + method: "POST", + pattern: "/api/runs/:runId/feedback/submit", + handler: async (_req, res, params) => { + const runId = requireRunId(res, params.runId); + if (!runId) return; + if (!runExists(db, runId)) { + writeJson(res, 404, { error: `Run ${runId} not found` }); + return; + } + + const submission = submitDraftFeedback(db, runId); + if (!submission) { + writeJson(res, 400, { error: "No draft feedback comments to submit" }); + return; + } + + await opts.deliverSubmission?.(submission); + writeJson(res, 200, { submission }); + opts.onSubmitted?.(); + }, + }, + ]; +} + +function requireRunId( + res: Parameters[1], + runId: string | undefined, +): string | null { + if (runId) return runId; + writeJson(res, 400, { error: "Missing runId" }); + return null; +} + +function requireRunAndCommentId( + res: Parameters[1], + runId: string | undefined, + commentId: string | undefined, +): { runId: string; commentId: string } | null { + const resolvedRunId = requireRunId(res, runId); + if (!resolvedRunId) return null; + if (commentId) return { runId: resolvedRunId, commentId }; + writeJson(res, 400, { error: "Missing commentId" }); + return null; +} + +async function parseBody( + req: Parameters[0], + res: Parameters[1], + schema: { safeParse: (value: unknown) => { success: true; data: T } | { success: false } }, + errorMessage: string, +): 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: errorMessage }); + return null; + } + return parsed.data; +} + +function runExists(db: StageDb, runId: string): boolean { + const rows = db + .select({ id: chapterRun.id }) + .from(chapterRun) + .where(eq(chapterRun.id, runId)) + .limit(1) + .all(); + return rows.length > 0; +} + +function targetBelongsToRun(db: StageDb, runId: string, chapterId: string | undefined): boolean { + if (!chapterId) return true; + const rows = db + .select({ id: chapter.id }) + .from(chapter) + .where(and(eq(chapter.id, chapterId), eq(chapter.runId, runId))) + .limit(1) + .all(); + return rows.length > 0; +} + +function findComment( + db: StageDb, + runId: string, + commentId: string, +): FeedbackCommentRow | undefined { + const [row] = db + .select() + .from(feedbackComment) + .where(and(eq(feedbackComment.id, commentId), eq(feedbackComment.runId, runId))) + .limit(1) + .all(); + return row; +} + +function submitDraftFeedback(db: StageDb, runId: string): FeedbackSubmission | null { + const submittedAt = new Date().toISOString(); + const submissionId = randomUUID(); + + return db.transaction((tx) => { + const drafts = tx + .select({ id: feedbackComment.id }) + .from(feedbackComment) + .where( + and( + eq(feedbackComment.runId, runId), + eq(feedbackComment.status, FEEDBACK_COMMENT_STATUS.DRAFT), + ), + ) + .orderBy(asc(feedbackComment.createdAt)) + .all(); + if (drafts.length === 0) return null; + + const draftIds = drafts.map((row) => row.id); + const rows = tx + .update(feedbackComment) + .set({ + status: FEEDBACK_COMMENT_STATUS.SUBMITTED, + submittedAt, + submissionId, + }) + .where(inArray(feedbackComment.id, draftIds)) + .returning() + .all(); + const byId = new Map(rows.map((row) => [row.id, row])); + const comments = draftIds.map((id) => { + const row = byId.get(id); + if (!row) throw new Error(`Missing submitted feedback row ${id}`); + return mapFeedbackComment(row); + }); + return { id: submissionId, runId, submittedAt, comments }; + }); +} + +function mapFeedbackComment(row: FeedbackCommentRow): FeedbackComment { + return { + id: row.id, + runId: row.runId, + target: row.target, + body: row.body, + status: row.status, + createdAt: row.createdAt.toISOString(), + updatedAt: row.updatedAt.toISOString(), + submittedAt: row.submittedAt, + submissionId: row.submissionId, + }; +} diff --git a/packages/cli/src/show.ts b/packages/cli/src/show.ts index ff3847c..80317f9 100644 --- a/packages/cli/src/show.ts +++ b/packages/cli/src/show.ts @@ -1,12 +1,13 @@ import { readFileSync } from "node:fs"; import path from "node:path"; -import open from "open"; import { buildOtherChangesChapter } from "./build-other-changes.js"; import { closeDb, getDb } from "./db/client.js"; import { parseGitDiff } from "./diff-parser.js"; +import { createFeedbackSink } from "./feedback-sink.js"; import { filterFilesForLlm, loadStageIgnore } from "./filter-files.js"; import { type ResolveScopeOptions, readRepoContext, readRepoRoot, resolveScope } from "./git.js"; import { diffRoutes } from "./routes/diff.js"; +import { feedbackRoutes } from "./routes/feedback.js"; import { pullRequestRoutes } from "./routes/pull-request.js"; import { pullRequestMutationRoutes } from "./routes/pull-request-mutations.js"; import { runRoutes } from "./routes/runs.js"; @@ -32,13 +33,23 @@ export async function show( ): Promise { const db = getDb(); const chaptersFile = loadChaptersFile(jsonPath, base, workingTreeRef, refs, compare); - const { runId } = insertChaptersFile(db, chaptersFile, readRepoContext()); + const repoContext = readRepoContext(); + const { runId } = insertChaptersFile(db, chaptersFile, repoContext); + const feedbackSink = createFeedbackSink(repoContext.root, runId); + let markFeedbackSubmitted: () => void = () => {}; + const feedbackSubmitted = new Promise((resolve) => { + markFeedbackSubmitted = resolve; + }); const handle = await startServer({ routes: [ ...runRoutes(db), ...viewStateRoutes(db), ...diffRoutes(db), + ...feedbackRoutes(db, { + deliverSubmission: feedbackSink.deliver, + onSubmitted: markFeedbackSubmitted, + }), ...pullRequestRoutes(db), ...pullRequestMutationRoutes(db), ], @@ -46,16 +57,11 @@ export async function show( const { port } = handle; const url = `http://${LOOPBACK_HOST}:${port}/runs/${encodeURIComponent(runId)}`; - process.stdout.write(`Listening on ${url}\n`); + process.stdout.write(`Review URL: ${url}\n`); + process.stdout.write(`Feedback file: ${feedbackSink.feedbackFilePath}\n`); process.stdout.write("Press Ctrl+C to exit.\n"); - try { - await open(url); - } catch { - // URL is on stdout — user can navigate manually. - } - - await waitForShutdownSignal(); + await waitForShutdownOrFeedback(feedbackSubmitted); await handle.close(); closeDb(); @@ -271,15 +277,19 @@ function sanitizeLineRefs( }); } -function waitForShutdownSignal(): Promise { +function waitForShutdownOrFeedback(feedbackSubmitted: Promise): Promise { return new Promise((resolve) => { - const cleanup = () => { - process.removeListener("SIGINT", cleanup); - process.removeListener("SIGTERM", cleanup); + let settled = false; + const finish = () => { + if (settled) return; + settled = true; + process.removeListener("SIGINT", finish); + process.removeListener("SIGTERM", finish); resolve(); }; - process.once("SIGINT", cleanup); - process.once("SIGTERM", cleanup); + process.once("SIGINT", finish); + process.once("SIGTERM", finish); + feedbackSubmitted.then(finish, finish); }); } diff --git a/packages/types/package.json b/packages/types/package.json index a2260c4..c0b57a0 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -8,6 +8,7 @@ ".": "./src/index.ts", "./chapters": "./src/chapters.ts", "./diff": "./src/diff.ts", + "./feedback": "./src/feedback.ts", "./parsed-diff": "./src/parsed-diff.ts", "./prologue": "./src/prologue.ts", "./pull-request": "./src/pull-request.ts", diff --git a/packages/types/src/feedback.ts b/packages/types/src/feedback.ts new file mode 100644 index 0000000..a11968c --- /dev/null +++ b/packages/types/src/feedback.ts @@ -0,0 +1,102 @@ +import { z } from "zod"; +import { DIFF_SIDE } from "./chapters.ts"; + +export const FEEDBACK_TARGET_TYPE = { + FILE: "file", + LINE: "line", +} as const; +export type FeedbackTargetType = (typeof FEEDBACK_TARGET_TYPE)[keyof typeof FEEDBACK_TARGET_TYPE]; + +export const FEEDBACK_COMMENT_STATUS = { + DRAFT: "draft", + SUBMITTED: "submitted", +} as const; +export type FeedbackCommentStatus = + (typeof FEEDBACK_COMMENT_STATUS)[keyof typeof FEEDBACK_COMMENT_STATUS]; + +export const FEEDBACK_BODY_MAX_LENGTH = 20_000; + +export const feedbackLineRangeSchema = z + .strictObject({ + side: z.enum(DIFF_SIDE), + startLine: z.number().int().positive(), + endLine: z.number().int().positive(), + endSide: z.enum(DIFF_SIDE).optional(), + }) + .refine((v) => v.startLine <= v.endLine, { + message: "endLine must be greater than or equal to startLine", + path: ["endLine"], + }); +export type FeedbackLineRange = z.infer; + +const chapterIdSchema = z.string().min(1).optional(); + +export const feedbackFileTargetSchema = z.strictObject({ + type: z.literal(FEEDBACK_TARGET_TYPE.FILE), + filePath: z.string().min(1), + chapterId: chapterIdSchema, +}); +export type FeedbackFileTarget = z.infer; + +export const feedbackLineTargetSchema = z.strictObject({ + type: z.literal(FEEDBACK_TARGET_TYPE.LINE), + filePath: z.string().min(1), + chapterId: chapterIdSchema, + range: feedbackLineRangeSchema, +}); +export type FeedbackLineTarget = z.infer; + +export const feedbackCommentTargetSchema = z.discriminatedUnion("type", [ + feedbackFileTargetSchema, + feedbackLineTargetSchema, +]); +export type FeedbackCommentTarget = z.infer; + +const feedbackBodySchema = z.string().trim().min(1).max(FEEDBACK_BODY_MAX_LENGTH); + +export const CreateFeedbackCommentBodySchema = z.strictObject({ + target: feedbackCommentTargetSchema, + body: feedbackBodySchema, +}); +export type CreateFeedbackCommentBody = z.infer; + +export const UpdateFeedbackCommentBodySchema = z.strictObject({ + body: feedbackBodySchema, +}); +export type UpdateFeedbackCommentBody = z.infer; + +export const FeedbackCommentSchema = z.strictObject({ + id: z.string(), + runId: z.string(), + target: feedbackCommentTargetSchema, + body: z.string(), + status: z.enum(FEEDBACK_COMMENT_STATUS), + createdAt: z.string(), + updatedAt: z.string(), + submittedAt: z.string().nullable(), + submissionId: z.string().nullable(), +}); +export type FeedbackComment = z.infer; + +export const FeedbackCommentsResponseSchema = z.strictObject({ + comments: z.array(FeedbackCommentSchema), +}); +export type FeedbackCommentsResponse = z.infer; + +export const FeedbackCommentResponseSchema = z.strictObject({ + comment: FeedbackCommentSchema, +}); +export type FeedbackCommentResponse = z.infer; + +export const FeedbackSubmissionSchema = z.strictObject({ + id: z.string(), + runId: z.string(), + submittedAt: z.string(), + comments: z.array(FeedbackCommentSchema), +}); +export type FeedbackSubmission = z.infer; + +export const FeedbackSubmissionResponseSchema = z.strictObject({ + submission: FeedbackSubmissionSchema, +}); +export type FeedbackSubmissionResponse = z.infer; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 715cf6b..dad5553 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -1,5 +1,6 @@ export * from "./chapters.ts"; export * from "./diff.ts"; +export * from "./feedback.ts"; export * from "./parsed-diff.ts"; export * from "./prologue.ts"; export * from "./pull-request.ts"; diff --git a/packages/web/src/components/chapter/chapter-file-list.tsx b/packages/web/src/components/chapter/chapter-file-list.tsx index 1118033..5a49a2e 100644 --- a/packages/web/src/components/chapter/chapter-file-list.tsx +++ b/packages/web/src/components/chapter/chapter-file-list.tsx @@ -3,13 +3,11 @@ import { FileFilterInput } from "@/components/files/file-filter-input"; import { FileTree, type ViewedConfig } from "@/components/files/file-tree"; import { FILE_VIEWED_STATE, type PullRequestFile } from "@/lib/diff-types"; -// The CLI has no review comments, so the tree never renders comment badges. -const NO_COMMENT_COUNTS: Map = new Map(); - interface ChapterFileListProps { files: PullRequestFile[]; focusedFilePath?: string; viewedPathSet: ReadonlySet; + commentCountsByPath: Map; onToggleFileViewed: (filePath: string) => void; onSelectFile: (filePath: string) => void; } @@ -18,6 +16,7 @@ export function ChapterFileList({ files, focusedFilePath, viewedPathSet, + commentCountsByPath, onToggleFileViewed, onSelectFile, }: ChapterFileListProps) { @@ -47,7 +46,7 @@ export function ChapterFileList({ focusedFilePath={focusedFilePath} onSelectFile={onSelectFile} viewed={viewed} - commentCountsByPath={NO_COMMENT_COUNTS} + commentCountsByPath={commentCountsByPath} filter={filter} /> diff --git a/packages/web/src/components/chapter/chapter-side-panel.tsx b/packages/web/src/components/chapter/chapter-side-panel.tsx index 9278f15..64f153a 100644 --- a/packages/web/src/components/chapter/chapter-side-panel.tsx +++ b/packages/web/src/components/chapter/chapter-side-panel.tsx @@ -22,6 +22,7 @@ interface ChapterSidePanelProps { viewedChapterIds: ReadonlySet; checkedKeyChangeIds: ReadonlySet; viewedFilePathSet: ReadonlySet; + feedbackCountsByPath: Map; focusedKeyChangeId: string | null; onToggleChapterViewed: (externalId: string) => void; onToggleKeyChangeChecked: (keyChangeId: string) => void; @@ -39,6 +40,7 @@ export function ChapterSidePanel({ viewedChapterIds, checkedKeyChangeIds, viewedFilePathSet, + feedbackCountsByPath, focusedKeyChangeId, onToggleChapterViewed, onToggleKeyChangeChecked, @@ -102,6 +104,7 @@ export function ChapterSidePanel({ files={files} focusedFilePath={focusedFilePath} viewedPathSet={viewedFilePathSet} + commentCountsByPath={feedbackCountsByPath} onToggleFileViewed={onToggleFileViewed} onSelectFile={onSelectFile} /> diff --git a/packages/web/src/components/chapter/pierre-diff-viewer.tsx b/packages/web/src/components/chapter/pierre-diff-viewer.tsx index 278a2a8..b714543 100644 --- a/packages/web/src/components/chapter/pierre-diff-viewer.tsx +++ b/packages/web/src/components/chapter/pierre-diff-viewer.tsx @@ -1,11 +1,14 @@ import { + type DiffLineAnnotation, type FileDiffMetadata, getSingularPatch, type Hunk, type SelectedLineRange, } from "@pierre/diffs"; import { FileDiff, PatchDiff } from "@pierre/diffs/react"; -import { useDeferredValue, useEffect, useMemo, useRef, useState } from "react"; +import type { FeedbackComment } from "@stagereview/types/feedback"; +import { useCallback, useDeferredValue, useEffect, useMemo, useRef, useState } from "react"; +import { FeedbackCommentCard } from "@/components/feedback"; import { type AnnotatedLineRef, COMMENT_SIDE, @@ -13,6 +16,7 @@ import { type LineRef, SIDE_TO_DIFF, } from "@/lib/diff-types"; +import { feedbackLineRangeForGutterClick } from "@/lib/feedback-targets"; import { resolveSyntaxTheme } from "@/lib/syntax-themes"; import { useDiffSettings } from "@/lib/use-diff-settings"; import { LineHighlightOverlay } from "./hunk-highlight-overlay"; @@ -93,6 +97,10 @@ type PierreDiffViewerProps = { onMarkKeyChangeChecked?: (keyChangeId: string) => void; onUnmarkKeyChangeChecked?: (keyChangeId: string) => void; onFocusKeyChange?: (keyChangeId: string | null, scrollTarget?: LineRef | null) => void; + feedbackComments?: FeedbackComment[]; + onCreateLineFeedback?: (lineRange: SelectedLineRange) => void; + onEditFeedback?: (comment: FeedbackComment) => void; + onDeleteFeedback?: (comment: FeedbackComment) => void; /** Force a specific theme. Defaults to detecting `.dark` on ``. */ appTheme?: AppTheme; } & ({ patch: string; fileDiff?: never } | { patch?: never; fileDiff: FileDiffMetadata }); @@ -113,6 +121,10 @@ export function PierreDiffViewer({ onMarkKeyChangeChecked, onUnmarkKeyChangeChecked, onFocusKeyChange, + feedbackComments = [], + onCreateLineFeedback, + onEditFeedback, + onDeleteFeedback, appTheme: appThemeProp, }: PierreDiffViewerProps) { const appTheme = useAppTheme(appThemeProp); @@ -131,6 +143,7 @@ export function PierreDiffViewer({ const deferredExpandUnchanged = useDeferredValue(expandUnchanged); const diffContainerRef = useRef(null); + const selectedFeedbackRangeRef = useRef(null); const focusedLineRefs = useMemo(() => { if (!focusedLineRefsByFile || !filePath) return undefined; @@ -142,6 +155,19 @@ export function PierreDiffViewer({ return allLineRefsByFile.get(filePath); }, [allLineRefsByFile, filePath]); + const handleLineSelected = useCallback((lineRange: SelectedLineRange | null) => { + selectedFeedbackRangeRef.current = lineRange; + }, []); + + const handleGutterUtilityClick = useCallback( + (lineRange: SelectedLineRange) => { + onCreateLineFeedback?.( + feedbackLineRangeForGutterClick(lineRange, selectedFeedbackRangeRef.current), + ); + }, + [onCreateLineFeedback], + ); + const options = useMemo( () => ({ theme: resolveSyntaxTheme(deferredSyntaxTheme, appTheme), @@ -156,7 +182,10 @@ export function PierreDiffViewer({ expansionLineCount: 20, overflow: deferredWrap ? ("wrap" as const) : ("scroll" as const), enableLineSelection: true, - enableHoverUtility: false, + enableGutterUtility: onCreateLineFeedback !== undefined, + lineHoverHighlight: onCreateLineFeedback ? ("number" as const) : ("disabled" as const), + onLineSelected: handleLineSelected, + onGutterUtilityClick: onCreateLineFeedback ? handleGutterUtilityClick : undefined, }), [ appTheme, @@ -168,6 +197,9 @@ export function PierreDiffViewer({ deferredWrap, deferredLineNumbers, deferredExpandUnchanged, + onCreateLineFeedback, + handleLineSelected, + handleGutterUtilityClick, ], ); @@ -176,6 +208,40 @@ export function PierreDiffViewer({ selectedLines: selectedLinesProp ?? null, }; + const feedbackLineAnnotations = useMemo[]>( + () => + feedbackComments.flatMap((comment) => { + if (comment.target.type !== "line") return []; + return [ + { + side: comment.target.range.endSide ?? comment.target.range.side, + lineNumber: comment.target.range.endLine, + metadata: comment, + }, + ]; + }), + [feedbackComments], + ); + + const renderFeedbackAnnotation = useCallback( + (annotation: DiffLineAnnotation) => ( +
+ +
+ ), + [onEditFeedback, onDeleteFeedback], + ); + + const feedbackProps = { + lineAnnotations: feedbackLineAnnotations, + renderAnnotation: renderFeedbackAnnotation, + }; + // Only mount the overlay when this file actually has refs to highlight. // The overlay's click-listener effect polls for Pierre's shadow root on // mount, so leaving it on for every diff (e.g. plain /files view, chapter @@ -200,7 +266,7 @@ export function PierreDiffViewer({ className="@container/diff relative isolate overflow-hidden rounded-b-lg border-x border-b border-border" ref={diffContainerRef} > - + {overlay} ); @@ -211,12 +277,14 @@ export function PierreDiffViewer({ className="@container/diff relative isolate overflow-hidden rounded-b-lg border-x border-b border-border" ref={diffContainerRef} > - + {overlay} ); } +function noopFeedback(_comment: FeedbackComment): void {} + /** * Re-exported helper for chapter container components: derive the addition-side * line range that covers a key change's hunks. Uses {@link getVisibleLineRange} diff --git a/packages/web/src/components/feedback/index.tsx b/packages/web/src/components/feedback/index.tsx new file mode 100644 index 0000000..7fcbdfd --- /dev/null +++ b/packages/web/src/components/feedback/index.tsx @@ -0,0 +1,212 @@ +import type { + CreateFeedbackCommentBody, + FeedbackComment, + FeedbackCommentTarget, +} from "@stagereview/types/feedback"; +import { Loader2, MessageSquare, Pencil, Trash2 } from "lucide-react"; +import { useEffect, useId, useState } from "react"; +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; +import { cn } from "@/lib/utils"; + +export type FeedbackComposerState = + | { + mode: "create"; + target: CreateFeedbackCommentBody["target"]; + } + | { + mode: "edit"; + comment: FeedbackComment; + }; + +interface FeedbackComposerDialogProps { + state: FeedbackComposerState | null; + isSaving: boolean; + onSubmit: (body: string) => Promise; + onClose: () => void; +} + +export function FeedbackComposerDialog({ + state, + isSaving, + onSubmit, + onClose, +}: FeedbackComposerDialogProps) { + const [body, setBody] = useState(""); + const textareaId = useId(); + const open = state !== null; + const target = state?.mode === "create" ? state.target : state?.comment.target; + const trimmed = body.trim(); + + useEffect(() => { + if (!state) { + setBody(""); + return; + } + setBody(state.mode === "edit" ? state.comment.body : ""); + }, [state]); + + const handleSubmit = async () => { + if (!trimmed || isSaving) return; + await onSubmit(trimmed); + onClose(); + }; + + return ( + !nextOpen && onClose()}> + + + {state?.mode === "edit" ? "Edit feedback" : "Add feedback"} + + {target ? formatFeedbackTarget(target) : "Feedback"} + + +
+ +