diff --git a/.changeset/review-anchors.md b/.changeset/review-anchors.md new file mode 100644 index 0000000..ee2e710 --- /dev/null +++ b/.changeset/review-anchors.md @@ -0,0 +1,7 @@ +--- +"@foldworks/diff-viewer": minor +"@foldworks/review": patch +--- + +Map review anchors across revisions conservatively and expand supplied context +with stale-gap validation; document outdated thread composition. diff --git a/.changeset/review-chrome.md b/.changeset/review-chrome.md new file mode 100644 index 0000000..041c49d --- /dev/null +++ b/.changeset/review-chrome.md @@ -0,0 +1,7 @@ +--- +"@foldworks/review": minor +"@foldworks/ui": minor +--- + +Add changed-file navigation, checks, merge requirements and change-request +headers. Extend checkable Tree composition and timeline activity body slots. diff --git a/.changeset/review-diff-input.md b/.changeset/review-diff-input.md new file mode 100644 index 0000000..6b48a96 --- /dev/null +++ b/.changeset/review-diff-input.md @@ -0,0 +1,6 @@ +--- +"@foldworks/diff-viewer": minor +--- + +Produce and serialize text diffs; add intraline markup, full-row line slots, +annotations and a semantic highlighting hook. diff --git a/.changeset/review-reference.md b/.changeset/review-reference.md new file mode 100644 index 0000000..01bda1f --- /dev/null +++ b/.changeset/review-reference.md @@ -0,0 +1,11 @@ +--- +"@foldworks/review": patch +"@foldworks/diff-viewer": patch +"@foldworks/diagram": patch +--- + +Exercise configuration review end to end in a persistent local reference host. +Retain suggestion drafts through saves, read current replacement text on submit, +restore composer focus after commit, and harden patch and highlight rendering. + +Use the current Foldkit subscription lifting API in the graph primitive. diff --git a/.changeset/review-suggestions.md b/.changeset/review-suggestions.md new file mode 100644 index 0000000..e979736 --- /dev/null +++ b/.changeset/review-suggestions.md @@ -0,0 +1,6 @@ +--- +"@foldworks/review": minor +--- + +Add checked, atomic suggested changes, pending comment display, controlled +review submission and reviewer status views. diff --git a/.changeset/review-threads.md b/.changeset/review-threads.md new file mode 100644 index 0000000..1a215de --- /dev/null +++ b/.changeset/review-threads.md @@ -0,0 +1,6 @@ +--- +"@foldworks/review": minor +--- + +Add generic review-thread schemas, acknowledged drafts, mutation out-messages, +accessible thread, composer and grouped list views. diff --git a/.changeset/review-values.md b/.changeset/review-values.md new file mode 100644 index 0000000..2f89932 --- /dev/null +++ b/.changeset/review-values.md @@ -0,0 +1,6 @@ +--- +"@foldworks/diff-viewer": minor +--- + +Compare decoded configuration values by path and array identity, render semantic +value changes, and map UTF-16 diagnostics to diff annotations. diff --git a/README.md b/README.md index 9948e8a..c69288f 100644 --- a/README.md +++ b/README.md @@ -28,13 +28,13 @@ contract: ## The stack -| Layer | What it provides | Packages | -| -------------------------- | --------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | -| 05. Workbenches | Complete tools composed from the layers below | `agent`, `generative-ui`, `codebase` | -| 04. Application primitives | Product surfaces with models, messages, and updates included | `editor`, `code-editor`, `outliner`, `data-table`, `data-grid`, `query-builder`, and more | -| 03. Behaviors | Headless engines that more than one surface shares | `keyboard`, `history`, `text-intelligence`, `diagram`, `pdf` | -| 02. Components | Accessible controls, overlays, and application chrome | `ui`, `sidebar` | -| 01. Tokens | Semantic CSS variables in light and dark, with Shadcn, Polaris, Apple, and other themes | `ui` | +| Layer | What it provides | Packages | +| -------------------------- | --------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- | +| 05. Workbenches | Complete tools composed from the layers below | `agent`, `generative-ui`, `codebase` | +| 04. Application primitives | Product surfaces with models, messages, and updates included | `editor`, `code-editor`, `outliner`, `data-table`, `data-grid`, `query-builder`, `review`, and more | +| 03. Behaviors | Headless engines that more than one surface shares | `keyboard`, `history`, `text-intelligence`, `diagram`, `pdf` | +| 02. Components | Accessible controls, overlays, and application chrome | `ui`, `sidebar` | +| 01. Tokens | Semantic CSS variables in light and dark, with Shadcn, Polaris, Apple, and other themes | `ui` | ### Components @@ -82,6 +82,7 @@ contract: - [`@foldworks/workflow`](./packages/workflow): recursive workflows, branch-aware operations, layout, and drag-and-drop primitives, built on the `@foldworks/diagram` scene contract. +- [`@foldworks/review`](./packages/review): review threads, drafts, and host-owned change-request workflows. - [`@foldworks/diff-viewer`](./packages/diff-viewer): a controlled unified and split diff renderer with line selection for code review workflows. - [`@foldworks/pdf-viewer`](./packages/pdf-viewer): read-only PDF viewing diff --git a/apps/demo/README.md b/apps/demo/README.md index 7f36a5f..5bca8bf 100644 --- a/apps/demo/README.md +++ b/apps/demo/README.md @@ -4,7 +4,7 @@ A browser-only demo app built with Foldkit, StyleX, and Vite. The homepage introduces the Foldworks package family, shows a live component composition, and explains the boundary between UI components and application primitives. Use the sidebar to move between every package showcase, including the interactive -diff review prototype at `/diff-viewer`. +local change-request reference at `/diff-viewer`. Foldkit's bidirectional router owns navigational state. The project overview lives at `/`; demos live at `/ui-kit`, `/editor`, `/codebase`, `/data-table`, `/data-grid`, `/query-builder`, @@ -172,3 +172,45 @@ The `/ui-kit` Desktop workspace demonstrates shared keyboard commands, measured breadcrumb/action rows, inline editing, token entry, a nested inspector, context menus, modal variants and a controlled date input. See [the integration guide](../../packages/ui/docs/desktop.md). + +## Local change-request reference + +`/diff-viewer` implements edit → diff → change request → pending line comments → +suggestions → checks → approve → merge → history. It has two synthetic JSON +configuration files validated by Effect schemas. All transport, identity, +persistence and policy are local demo host code; there is no external repository +or Git provider integration. + +Change checkout to true and rollout to 100 in CodeEditor (a string for checkout +shows a schema diagnostic). Open the request and switch to the local reviewer. +Comment on new line 4, then select new lines 3–4 and suggest: + +```json + "rollout": 25, + "reviewedBy": "Local reviewer" +``` + +Submit Request changes, switch to author and apply the suggestion. Its thread +is reanchored to the replacement revision; the old note discussion is outdated. +Approve as the reviewer, switch back to author, then merge. Local history, +comments, source text, review drafts and viewed files survive reload. Approval +only counts while its captured configuration text matches the working files. The example policy excludes outdated threads from +blocking requirements. Failed storage writes report an error and retain the +current in-memory session; this adapter has no multi-tab concurrency control. + +Keyboard users activate a gutter with Enter, extend an existing selection with +Shift+Arrow, save with Mod+Enter, and close with Escape to return to the gutter. +Closing retains drafts for the same anchor and revision. Thread lists use one +tab stop with Arrow/Home/End and Enter to reveal an anchor. Changed-file trees +use Space for viewed state. The reference includes local dark appearance and a +single-column narrow layout. + +Run the focused scenario with: + +```sh +pnpm --filter @foldworks/demo exec vitest run --config vitest.e2e.config.ts -t 'change request reference' +``` + +`pnpm test:e2e` also runs it. Documentation screenshots (animations disabled, not +pixel comparisons) are saved under `.context/demo-screenshots/change-request-*.png`. +Reset simulation clears this example's saved state. diff --git a/apps/demo/e2e/diff-viewer.scenarios.ts b/apps/demo/e2e/diff-viewer.scenarios.ts index 55a5d69..168025a 100644 --- a/apps/demo/e2e/diff-viewer.scenarios.ts +++ b/apps/demo/e2e/diff-viewer.scenarios.ts @@ -1,42 +1,183 @@ import type { Page } from "playwright"; import { describe, expect, it } from "vitest"; - +import { proposedFlags, suggestedFlags } from "../src/diff-viewer/fixture"; +const source = (page: Page) => + page.getByRole("textbox", { name: "Configuration source", exact: true }); +const diff = (page: Page) => page.locator(".cr-demo__diff > .fk-diff-viewer"); +const ready = async (page: Page) => + expect + .poll(() => + source(page).evaluate( + (element) => + (element as HTMLTextAreaElement & { foldkitNative?: { status: string } }).foldkitNative + ?.status, + ), + ) + .toBe("Ready"); export const diffViewerScenarios = ( getPage: () => Page, appUrl: string, screenshot: (name: string) => Promise, ): void => { - describe("diff review", () => { - it("changes layout, adds a line comment, and tracks viewed files", async () => { + describe("change request reference", () => { + it("edits, validates, requests changes, applies suggestions, approves, merges and reloads history", async () => { const page = getPage(); await page.goto(`${appUrl}/diff-viewer`, { waitUntil: "networkidle" }); - - await expect.poll(() => page.locator('[data-diff-review-demo="true"]').isVisible()).toBe(true); - await expect.poll(() => page.getByRole("region", { name: /Changes in apps\/demo\/src\/review\/commands.ts/ }).isVisible()).toBe(true); - await page.getByRole("button", { name: "Unified diff" }).click(); - await expect.poll(() => page.locator(".fk-diff-viewer").getAttribute("data-mode")).toBe("unified"); - - const rangeStart = page.getByRole("button", { name: "Comment on new line 20" }); - const rangeEnd = page.getByRole("button", { name: "Comment on new line 24" }); - const startBox = await rangeStart.boundingBox(); - const endBox = await rangeEnd.boundingBox(); - expect(startBox).not.toBeNull(); - expect(endBox).not.toBeNull(); - if (startBox === null || endBox === null) return; - await page.mouse.move(startBox.x + startBox.width / 2, startBox.y + startBox.height / 2); - await page.mouse.down(); - await page.mouse.move(endBox.x + endBox.width / 2, endBox.y + endBox.height / 2, { steps: 8 }); - await page.mouse.up(); - await expect.poll(() => page.getByText("New lines 20–24", { exact: true }).isVisible()).toBe(true); - await page.getByRole("textbox", { name: "Review comment" }).fill("Please add a regression test for the empty review path."); - await page.getByRole("button", { name: "Add comment" }).click(); - await expect.poll(() => page.getByText("Please add a regression test for the empty review path.", { exact: true }).isVisible()).toBe(true); - await expect.poll(() => page.getByText("New lines 20–24", { exact: true }).isVisible()).toBe(true); - - await page.getByRole("button", { name: /Mark .*commands.ts as viewed/ }).click(); - await expect.poll(() => page.locator(".review-demo__nav-footer").textContent()).toContain("2 of 5 viewed"); - await expect.poll(() => page.getByRole("button", { name: /Unmark .*commands.ts as viewed/ }).isVisible()).toBe(true); - await screenshot("diff-viewer-review"); - }); + await ready(page); + await expect + .poll(() => page.getByText("LOCAL SIMULATION", { exact: true }).isVisible()) + .toBe(true); + await source(page).fill(proposedFlags.replace('"checkout": true', '"checkout": "yes"')); + await expect.poll(() => page.locator(".native-token--issue").count()).toBeGreaterThan(0); + await expect + .poll(() => diff(page).locator('[data-annotation-tone="danger"]').count()) + .toBeGreaterThan(0); + await screenshot("change-request-01-schema-error"); + await source(page).fill(proposedFlags); + await expect.poll(() => page.locator(".native-token--issue").count()).toBe(0); + await page.getByRole("button", { name: "Semantic values", exact: true }).click(); + await expect + .poll(() => page.getByRole("region", { name: "Value changes" }).isVisible()) + .toBe(true); + await screenshot("change-request-02-values"); + await page.getByRole("button", { name: "Line diff", exact: true }).click(); + await page.getByRole("button", { name: "Open change request", exact: true }).click(); + const files = page.getByRole("tree", { name: "Changed files", exact: true }); + await files.focus(); + await files.press("End"); + await files.press("Enter"); + await expect + .poll(() => + page.getByRole("combobox", { name: "Configuration file", exact: true }).inputValue(), + ) + .toBe("config/access.json"); + await files.press(" "); + await expect + .poll(() => page.getByText("1 files viewed · 0 discussions", { exact: true }).isVisible()) + .toBe(true); + await page.getByRole("button", { name: /Expand .* unchanged lines/ }).click(); + await expect + .poll(() => + diff(page) + .getByRole("button", { name: "Comment on new line 1", exact: true }) + .isVisible(), + ) + .toBe(true); + await page + .getByRole("combobox", { name: "Configuration file", exact: true }) + .selectOption("config/flags.json"); + await page.getByRole("button", { name: "Act as reviewer", exact: true }).click(); + const noteLine = diff(page).getByRole("button", { + name: "Comment on new line 4", + exact: true, + }); + await noteLine.focus(); + await noteLine.press("Enter"); + const composer = page.getByRole("textbox", { name: "Review comment", exact: true }); + await expect + .poll(() => composer.evaluate((element) => element === document.activeElement)) + .toBe(true); + await composer.fill("Please remove this temporary note."); + await composer.press("Control+Enter"); + await expect + .poll(() => noteLine.evaluate((element) => element === document.activeElement)) + .toBe(true); + const start = diff(page).getByRole("button", { + name: /^(?:Comment on|Selected) new line 3$/, + }); + await start.focus(); + await start.press("Enter"); + await start.focus(); + await start.press("Shift+ArrowDown"); + await expect + .poll(() => page.getByText("New lines 3–4", { exact: true }).isVisible()) + .toBe(true); + await expect + .poll(() => composer.evaluate((element) => element === document.activeElement)) + .toBe(true); + await composer.fill("Keep the first rollout small and record the reviewer."); + await page.getByRole("button", { name: "Add suggestion", exact: true }).click(); + const replacement = page.getByRole("textbox", { name: "Suggested replacement", exact: true }); + await replacement.fill(suggestedFlags); + await replacement.press("Control+Enter"); + await expect + .poll(() => + page + .getByRole("form", { name: "Submit review", exact: true }) + .getByRole("status") + .textContent(), + ) + .toBe("2 pending comments"); + await page.getByRole("button", { name: "Request changes", exact: true }).click(); + await page.getByRole("button", { name: "Submit review", exact: true }).click(); + await screenshot("change-request-03-request-changes"); + await page.getByRole("button", { name: "Act as author", exact: true }).click(); + await page.getByRole("button", { name: "Apply suggestion", exact: true }).click(); + await expect.poll(() => page.locator('[data-anchor-state="outdated"]').count()).toBe(1); + await expect.poll(() => page.locator('[data-anchor-state="moved"]').count()).toBe(1); + await expect.poll(() => source(page).inputValue()).toContain('"rollout": 25'); + await screenshot("change-request-04-suggestion"); + await page.getByRole("button", { name: "Act as reviewer", exact: true }).click(); + await page.getByRole("button", { name: "Approve", exact: true }).click(); + await page.getByRole("button", { name: "Submit review", exact: true }).click(); + await page.getByRole("button", { name: "Act as author", exact: true }).click(); + await expect + .poll(() => + page.getByRole("button", { name: "Merge change request", exact: true }).isEnabled(), + ) + .toBe(true); + await page.setViewportSize({ width: 1600, height: 1200 }); + await page.locator(".cr-demo").evaluate((element) => { + let node: HTMLElement | null = element as HTMLElement; + while (node) { + node.scrollTop = 0; + node = node.parentElement; + } + }); + await screenshot("change-request-05-ready"); + await page.getByRole("button", { name: "Dark appearance", exact: true }).click(); + await screenshot("change-request-08-dark-desktop"); + await page.getByRole("button", { name: "Light appearance", exact: true }).click(); + await page.getByRole("button", { name: "Merge change request", exact: true }).click(); + await expect.poll(() => page.locator(".fk-review__header").textContent()).toContain("merged"); + await page.reload({ waitUntil: "networkidle" }); + await ready(page); + await expect + .poll(() => page.getByRole("region", { name: "Change request activity" }).textContent()) + .toContain("Configuration merged locally"); + await expect + .poll(() => source(page).inputValue()) + .toContain('"reviewedBy": "Local reviewer"'); + await screenshot("change-request-06-history"); + }, 90_000); + it("supports keyboard cancellation, narrow screens and dark appearance", async () => { + const page = getPage(); + await page.setViewportSize({ width: 390, height: 844 }); + await page.goto(`${appUrl}/diff-viewer`, { waitUntil: "networkidle" }); + await ready(page); + await source(page).fill(proposedFlags); + await page.getByRole("button", { name: "Open change request", exact: true }).click(); + await page.getByRole("button", { name: "Act as reviewer", exact: true }).click(); + const gutter = diff(page).getByRole("button", { + name: /^(?:Comment on|Selected) new line 3$/, + }); + await gutter.focus(); + await gutter.press("Enter"); + const composer = page.getByRole("textbox", { name: "Review comment", exact: true }); + await composer.fill("Draft survives closing"); + await composer.press("Escape"); + await expect + .poll(() => gutter.evaluate((element) => document.activeElement === element)) + .toBe(true); + await gutter.press("Enter"); + await expect.poll(() => composer.inputValue()).toBe("Draft survives closing"); + await composer.press("Escape"); + await page.getByRole("button", { name: "Dark appearance", exact: true }).click(); + await expect.poll(() => page.locator(".cr-demo").getAttribute("data-dark")).toBe("true"); + expect( + await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth), + ).toBe(true); + await screenshot("change-request-07-dark-mobile"); + }, 60_000); }); }; diff --git a/apps/demo/e2e/package-demo.scenarios.ts b/apps/demo/e2e/package-demo.scenarios.ts index f0fc59d..a452b8c 100644 --- a/apps/demo/e2e/package-demo.scenarios.ts +++ b/apps/demo/e2e/package-demo.scenarios.ts @@ -14,6 +14,7 @@ const expectedPackages = [ "@foldworks/form-builder", "@foldworks/workflow", "@foldworks/diff-viewer", + "@foldworks/review", "@foldworks/pdf-viewer", "@foldworks/pdf-annotator", "@foldworks/keyboard", diff --git a/apps/demo/package.json b/apps/demo/package.json index 1943368..e0075ec 100644 --- a/apps/demo/package.json +++ b/apps/demo/package.json @@ -39,7 +39,8 @@ "@stylexjs/stylex": "catalog:", "effect": "catalog:", "foldkit": "catalog:", - "pdf-lib": "catalog:" + "pdf-lib": "catalog:", + "@foldworks/review": "workspace:*" }, "devDependencies": { "@effect/platform-browser": "catalog:", diff --git a/apps/demo/src/app/subscriptions.ts b/apps/demo/src/app/subscriptions.ts index e5265e7..b7f5d24 100644 --- a/apps/demo/src/app/subscriptions.ts +++ b/apps/demo/src/app/subscriptions.ts @@ -131,8 +131,8 @@ const pdfAnnotator = Subscription.lift(PdfAnnotator.subscriptions)({ - when: (model) => model.route._tag === "DocumentAnalysis", - toChildModel: (model) => model.documentAnalysis, + read: (model) => + model.route._tag === "DocumentAnalysis" ? Option.some(model.documentAnalysis) : Option.none(), toParentMessage: (message) => Message.GotDocumentAnalysisMessage({ message }), }); diff --git a/apps/demo/src/app/view.ts b/apps/demo/src/app/view.ts index 045c092..316e743 100644 --- a/apps/demo/src/app/view.ts +++ b/apps/demo/src/app/view.ts @@ -474,7 +474,7 @@ const toolbar = (model: Model, h: HtmlBuilder): Html => { : demo === "Codebase" ? "Codebase workbench" : demo === "DiffViewer" - ? "Code review" + ? "Change request" : demo === "Workbench" ? "Workers workbench" : demo === "Workflow" @@ -516,7 +516,7 @@ const toolbar = (model: Model, h: HtmlBuilder): Html => { : demo === "Codebase" ? "Living documentation · source · history · Git diffs" : demo === "DiffViewer" - ? "Split and unified diffs · line comments · review progress" + ? "Local configuration · review · checks · merge history" : demo === "Workbench" ? "Inspect · Explain · Preview · Apply · History" : demo === "Workflow" @@ -571,12 +571,7 @@ const toolbar = (model: Model, h: HtmlBuilder): Html => { : demo === "Codebase" ? [toolbarBadge({ label: "Read-only fixture", tone: "info", dot: true }, h)] : demo === "DiffViewer" - ? [ - toolbarBadge( - { label: "Interactive prototype", tone: "info", dot: true }, - h, - ), - ] + ? [toolbarBadge({ label: "Local simulation", tone: "info", dot: true }, h)] : demo === "Workbench" ? [toolbarBadge({ label: "Reference workspace", dot: true }, h)] : demo === "DataTable" @@ -864,7 +859,7 @@ const documentTitle = (demo: Demo): string => Match.when("Agent", () => "Interactive agent · Foldworks"), Match.when("CodeEditor", () => "Code editor · Foldworks"), Match.when("Codebase", () => "Codebase workbench · Foldworks"), - Match.when("DiffViewer", () => "Code review · Foldworks"), + Match.when("DiffViewer", () => "Change request · Foldworks"), Match.when("Workbench", () => "Workers workbench · Foldworks"), Match.when("Home", () => "Foldworks · The UI standard library for Foldkit and StyleX"), Match.when("DataTable", () => "Data table · Foldworks"), diff --git a/apps/demo/src/diff-viewer/domain.ts b/apps/demo/src/diff-viewer/domain.ts new file mode 100644 index 0000000..aea80ee --- /dev/null +++ b/apps/demo/src/diff-viewer/domain.ts @@ -0,0 +1,108 @@ +import { diffFile, mapRange, type JsonValue, type RangeMapping } from "@foldworks/diff-viewer"; +import { highlight } from "@foldworks/code-editor"; +import { validate } from "@foldworks/code-editor/structured"; +import type { Check } from "@foldworks/review"; +import type { ChangedFiles } from "@foldworks/review"; +import type { Model, ReviewThread } from "./model"; +import { Flags, Access } from "./fixture"; + +export const currentFile = (model: Model) => model.files.find((f) => f.path === model.activePath)!; +export const fileDiff = (file: Model["files"][number]) => + diffFile({ path: file.path, oldText: file.baseText, newText: file.text, context: 3 }); +export const revisionKey = (model: Model) => + JSON.stringify(model.files.map((f) => [f.path, f.text])); +export const diagnostics = (file: Model["files"][number]) => + validate(file.path.endsWith("flags.json") ? Flags : Access, { + uri: file.path, + session: 0, + revision: 0, + languageId: "json", + text: file.text, + lineEnding: "lf", + }); +export const mappedThread = (model: Model, thread: ReviewThread): RangeMapping => { + const file = model.files.find((f) => f.path === thread.anchor.path); + if (!file) return { state: "outdated", reason: "unknown" }; + const from = diffFile({ + path: file.path, + oldText: thread.anchor.revisionText, + newText: thread.anchor.revisionText, + }); + const mapping = mapRange(from, thread.anchor, fileDiff(file)); + return mapping.state !== "outdated" && thread.anchor.originalText !== thread.anchor.revisionText + ? { ...mapping, state: "moved" } + : mapping; +}; +export const changedFiles = (model: Model): readonly ChangedFiles.ChangedFile[] => + model.files + .filter((f) => f.text !== f.baseText) + .map((file) => { + const diff = fileDiff(file), + issues = diagnostics(file); + return { + path: file.path, + status: diff.status, + additions: diff.additions, + deletions: diff.deletions, + viewed: model.viewedPaths.includes(file.path), + openThreads: model.threads.filter((t) => t.anchor.path === file.path && t.status === "open") + .length, + ...(issues.length ? { annotationTone: "danger" as const } : {}), + }; + }); +export const checks = (model: Model): readonly Check[] => { + const issues = model.files.flatMap(diagnostics); + let policy = false; + try { + const flags = JSON.parse(model.files.find((f) => f.path.endsWith("flags.json"))!.text); + const access = JSON.parse(model.files.find((f) => f.path.endsWith("access.json"))!.text); + policy = (!flags.checkout || flags.rollout <= 25) && access.requireApproval === true; + } catch { + /* invalid syntax is also a failed policy input */ + } + return [ + { + id: "schema", + name: "Local schema validation", + status: issues.length ? "failed" : "passed", + summary: issues.length + ? `${issues.length} configuration diagnostics` + : "Both files satisfy their Effect schemas.", + }, + { + id: "policy", + name: "Local rollout policy", + status: policy ? "passed" : "failed", + summary: "Illustrative policy: checkout rollout ≤25% and access changes require approval.", + }, + ]; +}; +export const openCurrentThreads = (model: Model) => + model.threads.filter((t) => t.status === "open" && mappedThread(model, t).state !== "outdated"); +export const approved = (model: Model) => + model.reviews.at(-1)?.verdict === "approve" && + model.reviews.at(-1)?.revision === revisionKey(model); +export const canMerge = (model: Model) => + model.actor === "author" && + model.status === "open" && + approved(model) && + checks(model).every((c) => c.status === "passed") && + !openCurrentThreads(model).length && + !model.threads.some((t) => t.comments.some((c) => c.pending)); +export const parseValue = (text: string): JsonValue | undefined => { + try { + return JSON.parse(text) as JsonValue; + } catch { + return undefined; + } +}; + +/** Adapt the editor lexer to the shared UTF-16 semantic-token vocabulary. */ +export const reviewHighlight = (content: string, language: string) => { + let offset = 0; + return (highlight(content, language)[0] ?? []).map((token) => { + const from = offset; + offset += token.text.length; + return { from, to: offset, kind: token.kind }; + }); +}; diff --git a/apps/demo/src/diff-viewer/fixture.ts b/apps/demo/src/diff-viewer/fixture.ts index 87fd19b..731f7a2 100644 --- a/apps/demo/src/diff-viewer/fixture.ts +++ b/apps/demo/src/diff-viewer/fixture.ts @@ -1,110 +1,70 @@ -import { parseUnifiedDiff } from "@foldworks/diff-viewer"; +import { Schema as S } from "effect"; +import { CodeEditor } from "@foldworks/code-editor"; +import { withSchema } from "@foldworks/code-editor/structured"; -export const reviewPatch = `diff --git a/apps/demo/src/review/commands.ts b/apps/demo/src/review/commands.ts -index 42b49ad..7c8e510 100644 ---- a/apps/demo/src/review/commands.ts -+++ b/apps/demo/src/review/commands.ts -@@ -18,7 +18,14 @@ export const submitReview = (review: Review) => - export const publishReview = async (review: Review, client: Client) => { -- await client.createReview(review); -+ const pending = review.comments.filter((comment) => !comment.resolved); -+ if (pending.length === 0) { -+ throw new Error("Add at least one unresolved comment before publishing"); -+ } -+ -+ await client.createReview({ ...review, comments: pending }); -+ metrics.increment("review.published", { disposition: review.disposition }); - }; -${" "} - export const discardReview = (id: string) => storage.remove(id); -@@ -48,6 +55,12 @@ export const summarizeReview = (review: Review): Summary => ({ - files: new Set(review.comments.map((comment) => comment.path)).size, - comments: review.comments.length, -+ blocking: review.comments.filter((comment) => comment.severity === "blocking").length, - }); -+ -+export const reviewIsReady = (review: Review): boolean => -+ review.comments.some((comment) => !comment.resolved) && -+ review.comments.every((comment) => comment.body.trim().length > 0); -diff --git a/apps/demo/src/review/panel.ts b/apps/demo/src/review/panel.ts -new file mode 100644 -index 0000000..d3a0183 ---- /dev/null -+++ b/apps/demo/src/review/panel.ts -@@ -0,0 +1,24 @@ -+import type { Html, HtmlBuilder } from "foldkit/html"; -+ -+import type { Review, ReviewMessage } from "./model"; -+ -+export const reviewPanel = ( -+ review: Review, -+ h: HtmlBuilder, -+): Html => h.aside([h.Class("review-panel")], [ -+ h.header([], [ -+ h.h2([], ["Review details"]), -+ h.span([], [\`\${review.comments.length} comments\`]), -+ ]), -+ h.div([h.Class("review-panel__progress")], [ -+ h.span([], [\`\${review.viewedFiles.length} / \${review.files.length} viewed\`]), -+ ]), -+ h.ul([], review.comments.map((comment) => h.li([], [ -+ h.strong([], [comment.author]), -+ h.p([], [comment.body]), -+ ]))), -+]); -diff --git a/packages/codebase/src/git.ts b/packages/codebase/src/git.ts -index 842f220..a8d96d1 100644 ---- a/packages/codebase/src/git.ts -+++ b/packages/codebase/src/git.ts -@@ -71,10 +71,16 @@ export const repositoryChanges = async (root: string) => { - const output = await git(root, ["status", "--short"]); -- return output.split("\\n").filter(Boolean).map(parseStatusLine); -+ return output -+ .split("\\n") -+ .filter(Boolean) -+ .map(parseStatusLine) -+ .sort((left, right) => left.path.localeCompare(right.path)); - }; -${" "} --export const diff = (root: string, scope: DiffScope) => -- git(root, scope === "staged" ? ["diff", "--cached"] : ["diff"]); -+export const diff = (root: string, scope: DiffScope, path?: string) => { -+ const args = scope === "staged" ? ["diff", "--cached"] : ["diff"]; -+ if (path !== undefined) args.push("--", path); -+ return git(root, args); -+}; -diff --git a/packages/diff-viewer/README.md b/packages/diff-viewer/README.md -new file mode 100644 -index 0000000..f79ab30 ---- /dev/null -+++ b/packages/diff-viewer/README.md -@@ -0,0 +1,13 @@ -+# Diff viewer -+ -+A controlled code review primitive for Foldkit applications. -+ -+## Design goals -+ -+- Keep repository access outside the renderer. -+- Support unified and side-by-side layouts. -+- Make every changed line keyboard reachable. -+- Keep review comments in application-owned state. -+- Render large patches incrementally. -+ -+The demo uses a static patch. Production adapters can provide data from local -+Git, GitHub, GitLab, or an agent workspace without changing the view contract. -diff --git a/apps/demo/src/styles.css b/apps/demo/src/styles.css -index f3b4a79..8b0e49e 100644 ---- a/apps/demo/src/styles.css -+++ b/apps/demo/src/styles.css -@@ -7,6 +7,7 @@ - @import "@foldworks/code-editor/styles.css"; - @import "@foldworks/data-grid/styles.css"; - @import "@foldworks/data-table/styles.css"; -+@import "@foldworks/diff-viewer/styles.css"; - @import "@foldworks/pdf-annotator/styles.css"; -${" "} - *, -`; - -export const reviewFiles = parseUnifiedDiff(reviewPatch); +export const Flags = S.Struct({ + checkout: S.Boolean, + rollout: S.Number.check(S.isBetween({ minimum: 0, maximum: 100 })), + retiredNote: S.optional(S.String), + reviewedBy: S.optional(S.String), +}); +export const Access = S.Struct({ admins: S.Array(S.String), requireApproval: S.Boolean }); +const FlagsEditor = withSchema(CodeEditor.implementation, Flags); +const AccessEditor = withSchema(CodeEditor.implementation, Access); +export const editorFor = (path: string) => + path === "config/flags.json" ? FlagsEditor : AccessEditor; +export const savedFlags = + '{\n "checkout": false,\n "rollout": 0,\n "retiredNote": "remove after rollout"\n}\n'; +export const proposedFlags = savedFlags + .replace('"checkout": false', '"checkout": true') + .replace('"rollout": 0', '"rollout": 100'); +export const suggestedFlags = ' "rollout": 25,\n "reviewedBy": "Local reviewer"'; +export const initialFiles = [ + { path: "config/flags.json", baseText: savedFlags, text: savedFlags }, + { + path: "config/access.json", + baseText: + JSON.stringify( + { + admins: [ + "author", + "operations", + "support", + "billing", + "security", + "compliance", + "analytics", + "growth", + "design", + "engineering", + ], + requireApproval: false, + }, + null, + 2, + ) + "\n", + text: + JSON.stringify( + { + admins: [ + "author", + "operations", + "support", + "billing", + "security", + "compliance", + "analytics", + "growth", + "design", + "engineering", + ], + requireApproval: true, + }, + null, + 2, + ) + "\n", + }, +]; +export const author = { id: "author", name: "Local author" }; +export const reviewer = { id: "reviewer", name: "Local reviewer" }; +export const storageKey = "foldworks.change-request.v1"; diff --git a/apps/demo/src/diff-viewer/message.ts b/apps/demo/src/diff-viewer/message.ts index e096c79..b3b39fc 100644 --- a/apps/demo/src/diff-viewer/message.ts +++ b/apps/demo/src/diff-viewer/message.ts @@ -1,17 +1,34 @@ import { Schema as S } from "effect"; import { defineMessageUnion } from "foldkit/message"; +import { CodeEditor } from "@foldworks/code-editor"; +import { ChangedFiles, ReviewThreads, Suggestion, Verdict } from "@foldworks/review"; +const Line = { path: S.String, side: S.Literals(["old", "new"]), line: S.Number }; export const Message = defineMessageUnion({ + GotEditorMessage: { message: CodeEditor.Message }, + GotThreadsMessage: { message: ReviewThreads.Message }, + GotFilesMessage: { message: ChangedFiles.Message }, SelectedFile: { path: S.String }, ChangedMode: { mode: S.Literals(["Unified", "Split"]) }, - SelectedLine: { path: S.String, side: S.Literals(["old", "new"]), line: S.Number }, - StartedSelection: { path: S.String, side: S.Literals(["old", "new"]), line: S.Number }, - ExtendedSelection: { path: S.String, side: S.Literals(["old", "new"]), line: S.Number, method: S.Literals(["Pointer", "Keyboard"]) }, + ToggledSemantic: {}, + ToggledDark: {}, + SelectedLine: Line, + StartedSelection: Line, + ExtendedSelection: { ...Line, method: S.Literals(["Pointer", "Keyboard"]) }, EndedSelection: {}, - ChangedDraft: { value: S.String }, - SubmittedComment: {}, - CancelledComment: {}, - ToggledViewed: { path: S.String, viewed: S.Boolean }, - ToggledResolved: { id: S.String }, + CancelledSelection: {}, + ExpandedGap: { id: S.String }, + ChangedViewed: { path: S.String, viewed: S.Boolean }, + SwitchedActor: { actor: S.Literals(["author", "reviewer"]) }, + OpenedRequest: {}, + AppliedSuggestion: { suggestion: Suggestion, threadId: S.String }, + ChangedSummary: { value: S.String }, + ChangedVerdict: { verdict: Verdict }, + SubmittedReview: {}, + ReranChecks: {}, + Merged: {}, + Reset: {}, + Persisted: { error: S.String }, + CompletedReveal: {}, }); export type Message = typeof Message.Type; diff --git a/apps/demo/src/diff-viewer/model.ts b/apps/demo/src/diff-viewer/model.ts index fed729a..a56e050 100644 --- a/apps/demo/src/diff-viewer/model.ts +++ b/apps/demo/src/diff-viewer/model.ts @@ -1,65 +1,110 @@ import { Schema as S } from "effect"; +import { CodeEditor } from "@foldworks/code-editor"; +import { ReviewThreads, ChangedFiles, Thread, Verdict } from "@foldworks/review"; +import { initialFiles, editorFor, storageKey } from "./fixture"; -import { reviewFiles } from "./fixture"; - -export const ReviewComment = S.Struct({ - id: S.String, +export const FileState = S.Struct({ path: S.String, baseText: S.String, text: S.String }); +export const Range = S.Struct({ path: S.String, side: S.Literals(["old", "new"]), startLine: S.Number, endLine: S.Number, - author: S.String, - body: S.String, - resolved: S.Boolean, }); -export type ReviewComment = typeof ReviewComment.Type; - +export const Anchor = S.Struct({ ...Range.fields, revisionText: S.String, originalText: S.String }); +export type Anchor = typeof Anchor.Type; +export const ReviewThread = Thread(Anchor); +export type ReviewThread = typeof ReviewThread.Type; +export const Activity = S.Struct({ + id: S.String, + title: S.String, + actor: S.String, + timestamp: S.String, + context: S.String, + kind: S.String, + changes: S.Array(S.Struct({ label: S.String, before: S.String, after: S.String })), +}); export const Model = S.Struct({ + files: S.Array(FileState), + editor: CodeEditor.Model, activePath: S.String, + threads: S.Array(ReviewThread), + reviewUi: ReviewThreads.Model, + filesUi: ChangedFiles.Model, + draftAnchors: S.Record(S.String, Anchor), + selection: S.NullOr(Range), + dragging: S.Boolean, mode: S.Literals(["Unified", "Split"]), + semantic: S.Boolean, viewedPaths: S.Array(S.String), - selectionPath: S.String, - selectionSide: S.Literals(["old", "new"]), - selectionStartLine: S.Number, - selectionEndLine: S.Number, - selectionDragging: S.Boolean, - draft: S.String, - comments: S.Array(ReviewComment), + expandedGaps: S.Array(S.String), + actor: S.Literals(["author", "reviewer"]), + status: S.Literals(["draft", "open", "merged", "closed"]), + summary: S.String, + verdict: Verdict, + reviews: S.Array(S.Struct({ verdict: Verdict, summary: S.String, revision: S.String })), + activity: S.Array(Activity), + nextId: S.Number, announcement: S.String, + storageError: S.String, + dark: S.Boolean, }); export type Model = typeof Model.Type; - -export const initialModel: Model = { - activePath: reviewFiles[0]?.path ?? "", - mode: "Split", - viewedPaths: [reviewFiles[1]?.path ?? ""].filter(Boolean), - selectionPath: "", - selectionSide: "new", - selectionStartLine: 0, - selectionEndLine: 0, - selectionDragging: false, - draft: "", - comments: [ - { - id: "comment-1", - path: reviewFiles[0]?.path ?? "", - side: "new", - startLine: 24, - endLine: 24, - author: "Maya Chen", - body: "Should this emit only after the API call succeeds? Otherwise retries could double-count published reviews.", - resolved: false, - }, - { - id: "comment-2", - path: reviewFiles[2]?.path ?? "", - side: "new", - startLine: 82, - endLine: 82, - author: "Noah Williams", - body: "Nice—scoping the patch at the Git boundary will keep large reviews responsive.", - resolved: true, - }, - ], - announcement: "Code review ready. Five files changed.", +export const freshModel = (): Model => ({ + files: initialFiles, + editor: editorFor(initialFiles[0]!.path).init({ + id: "change-request-editor", + uri: initialFiles[0]!.path, + languageId: "json", + text: initialFiles[0]!.text, + }), + activePath: initialFiles[0]!.path, + threads: [], + reviewUi: ReviewThreads.init({ id: "change-request-review" }), + filesUi: ChangedFiles.init({ + id: "change-request-files", + files: initialFiles.map((f) => ({ ...f, status: "modified", additions: 0, deletions: 0 })), + }), + draftAnchors: {}, + selection: null, + dragging: false, + mode: "Unified", + semantic: false, + viewedPaths: [], + expandedGaps: [], + actor: "author", + status: "draft", + summary: "", + verdict: "comment", + reviews: [], + activity: [], + nextId: 1, + announcement: "Local configuration workspace ready.", + storageError: "", + dark: false, +}); +const restore = (): Model => { + if (typeof localStorage === "undefined") return freshModel(); + try { + const text = localStorage.getItem(storageKey); + if (!text) return freshModel(); + const saved = S.decodeUnknownSync(Model)(JSON.parse(text)); + const file = saved.files.find((f) => f.path === saved.activePath) ?? saved.files[0]; + if (!file) return freshModel(); + return { + ...saved, + dragging: false, + editor: editorFor(file.path).init({ + id: "change-request-editor", + uri: file.path, + languageId: "json", + text: file.text, + }), + }; + } catch { + return { + ...freshModel(), + storageError: "Saved simulation could not be read. This session starts fresh.", + }; + } }; +export const initialModel = restore(); diff --git a/apps/demo/src/diff-viewer/styles.css b/apps/demo/src/diff-viewer/styles.css index 3e728f3..9520237 100644 --- a/apps/demo/src/diff-viewer/styles.css +++ b/apps/demo/src/diff-viewer/styles.css @@ -1,125 +1,289 @@ -.review-demo { - --review-border: var(--foldworks-ui-border, #e2e2e4); - --review-border-strong: var(--foldworks-ui-border-strong, #d3d3d6); - --review-surface: var(--foldworks-ui-surface, #fff); - --review-subtle: var(--foldworks-ui-surface-subtle, #f6f6f7); - --review-canvas: var(--foldworks-ui-canvas, #f7f7f8); - --review-text: var(--foldworks-ui-foreground, #202124); - --review-muted: var(--foldworks-ui-foreground-muted, #707176); - --review-accent: var(--foldworks-ui-selection-foreground, #16745a); - background: var(--review-canvas); - color: var(--review-text); +.cr-demo { + --native-editor-height: 180px; + --foldworks-ui-foreground: var(--foreground); + --foldworks-ui-foreground-muted: var(--muted-foreground); + --foldworks-ui-surface: var(--card); + --foldworks-ui-surface-subtle: var(--secondary); + --foldworks-ui-canvas: var(--background); + --foldworks-ui-border: var(--border); + --foldworks-ui-border-strong: var(--input); + color: var(--foreground); + background: var(--background); + padding: 24px; + min-height: 100%; + font: + 13px/1.5 Inter, + system-ui, + sans-serif; +} +.cr-demo[data-dark="true"] { + --background: #141719; + --foreground: #e9edef; + --card: #1b2023; + --card-foreground: #e9edef; + --secondary: #242b30; + --muted: #20272b; + --muted-foreground: #a5b0b8; + --border: #343e45; + --input: #43505a; + --accent: #2b383c; + --accent-foreground: #dbf3e9; + --ring: #77c8ad; + --primary: #78c9ad; + --primary-foreground: #102820; + color-scheme: dark; +} +.cr-demo__notice { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 12px; + padding: 10px 14px; + background: var(--secondary); + border: 1px solid var(--border); + border-radius: 8px; + margin-bottom: 16px; + color: var(--muted-foreground); +} +.cr-demo__notice strong { + font-size: 10px; + letter-spacing: 0.1em; + color: var(--foreground); +} +.cr-demo .fk-review__header { + border: 1px solid var(--border); + border-radius: 10px; + overflow: hidden; + margin-bottom: 18px; + background: var(--card); +} +.cr-demo .fk-review__header p { + margin: 8px 20px 14px; + color: var(--muted-foreground); +} +.cr-demo__toolbar { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: 8px; +} +.cr-demo__actor { + margin-inline-start: auto; + color: var(--muted-foreground); +} +.cr-demo__button, +.cr-demo .fk-review__panel button, +.cr-demo .fk-review__thread button, +.cr-demo .fk-review__composer button, +.cr-demo .fk-review__list button { + border: 1px solid var(--border); + background: var(--card); + color: var(--foreground); + border-radius: 6px; + padding: 6px 10px; + font: inherit; + cursor: pointer; +} +.cr-demo button:disabled { + opacity: 0.5; + cursor: default; +} +.cr-demo button:focus-visible, +.cr-demo [tabindex]:focus-visible, +.cr-demo input:focus-visible, +.cr-demo select:focus-visible, +.cr-demo textarea:focus-visible { + outline: 2px solid var(--ring); + outline-offset: 3px; +} +.cr-demo select, +.cr-demo input[type="search"], +.cr-demo .fk-review__panel textarea, +.cr-demo .fk-review__composer textarea { + border: 1px solid var(--input); + border-radius: 6px; + padding: 8px; + background: var(--card); + color: var(--foreground); + font: inherit; + max-width: 100%; +} +.cr-demo__hint { + color: var(--muted-foreground); + margin-block: 12px 20px; + max-width: 100ch; +} +.cr-demo__workspace { display: grid; - font-family: Inter, ui-sans-serif, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; - grid-template-rows: 44px minmax(0, 1fr); - height: 100%; - min-height: 0; + grid-template-columns: 220px minmax(0, 1fr) 300px; + gap: 20px; + align-items: start; +} +.cr-demo__navigation, +.cr-demo__canvas, +.cr-demo__inspector { min-width: 0; +} +.cr-demo__canvas { + display: grid; + gap: 24px; +} +.cr-demo__editor, +.cr-demo__diff { + background: var(--card); + border: 1px solid var(--border); + border-radius: 10px; overflow: hidden; + min-width: 0; } - -.review-demo button, -.review-demo textarea { font: inherit; } - -.review-demo__topbar { - align-items: center; - background: var(--review-surface); - border-bottom: 1px solid var(--review-border-strong); +.cr-demo__section-heading { display: flex; - gap: 16px; + flex-wrap: wrap; + gap: 10px; + align-items: center; justify-content: space-between; + padding: 12px 16px; + border-bottom: 1px solid var(--border); +} +.cr-demo h2, +.cr-demo h3 { + font-size: 14px; + font-weight: 650; + margin-block: 0 8px; +} +.cr-demo h2 { + margin: 0; +} +.cr-demo__section-heading > span { + color: var(--muted-foreground); + font-size: 11px; +} +.cr-demo .native-editor { + border: 0; + border-radius: 0; +} +.cr-demo .native-editor__viewport { + max-height: 260px; +} +.cr-demo .fk-diff-viewer { min-width: 0; - padding: 0 10px 0 14px; -} -.review-demo__crumbs { align-items: center; display: flex; font-size: 11px; gap: 6px; min-width: 0; overflow: hidden; white-space: nowrap; } -.review-demo__crumbs span { color: var(--review-muted); } -.review-demo__crumbs strong { overflow: hidden; text-overflow: ellipsis; } -.review-demo__topbar-actions { align-items: center; display: flex; flex: 0 0 auto; gap: 8px; } -.review-demo__position { color: var(--review-muted); font: 10px ui-monospace, SFMono-Regular, Menlo, monospace; } - -.review-demo__mode { background: var(--review-subtle); border: 1px solid var(--review-border); border-radius: 7px; display: flex; gap: 2px; padding: 2px; } -.review-demo__mode button { align-items: center; background: transparent; border: 0; border-radius: 5px; color: var(--review-muted); cursor: pointer; display: flex; font-size: 10px; font-weight: 650; gap: 5px; height: 25px; padding: 0 8px; } -.review-demo__mode button[data-active="true"] { background: var(--review-surface); box-shadow: 0 1px 2px rgba(20, 22, 26, .09); color: var(--review-text); } -.review-demo__mode button:focus-visible { outline: 2px solid var(--review-accent); } - -.review-demo__workspace { display: grid; grid-template-columns: 224px minmax(500px, 1fr) 270px; min-height: 0; min-width: 0; } -.review-demo__navigation, -.review-demo__inspector { background: var(--review-surface); min-height: 0; overflow: auto; } -.review-demo__navigation { border-right: 1px solid var(--review-border-strong); display: flex; flex-direction: column; } -.review-demo__inspector { border-left: 1px solid var(--review-border-strong); } - -.review-demo__nav-heading { border-bottom: 1px solid var(--review-border); padding: 14px 12px 11px; } -.review-demo__nav-heading > div { align-items: center; display: flex; justify-content: space-between; } -.review-demo__nav-heading strong { font-size: 12px; } -.review-demo__nav-heading span { background: var(--review-subtle); border-radius: 5px; color: var(--review-muted); font: 10px ui-monospace, monospace; padding: 2px 5px; } -.review-demo__nav-heading p { color: var(--review-muted); font-size: 10px; margin: 4px 0 0; } -.review-demo__progress-track { background: var(--review-border); height: 2px; } -.review-demo__progress-track span { background: var(--review-accent); display: block; height: 100%; transition: width 180ms ease; } -.review-demo__file-list { display: flex; flex: 1; flex-direction: column; gap: 2px; padding: 7px; } -.review-demo__file { align-items: center; background: transparent; border: 0; border-radius: 7px; color: var(--review-text); cursor: pointer; display: grid; gap: 7px; grid-template-columns: 19px minmax(0, 1fr) auto; min-height: 43px; padding: 5px 6px; text-align: left; width: 100%; } -.review-demo__file:hover { background: var(--review-subtle); } -.review-demo__file[data-active="true"] { background: color-mix(in srgb, var(--review-accent) 11%, var(--review-surface)); } -.review-demo__file:focus-visible { outline: 2px solid var(--review-accent); } -.review-demo__file-state { align-items: center; color: #a27504; display: flex; font: 700 10px ui-monospace, monospace; height: 19px; justify-content: center; } -.review-demo__file-state[data-status="added"] { color: #218354; } -.review-demo__file-copy { display: flex; flex-direction: column; min-width: 0; } -.review-demo__file-copy strong { font-size: 11px; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } -.review-demo__file-copy > span { color: var(--review-muted); direction: rtl; font-size: 9px; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } -.review-demo__file-stats { align-items: center; display: flex; font: 9px ui-monospace, monospace; gap: 4px; } -.review-demo__plus { color: #218354; } -.review-demo__minus { color: #c34d55; } -.review-demo__comment-count { align-items: center; background: #4f7bef; border-radius: 999px; color: #fff; display: flex; font-weight: 700; height: 15px; justify-content: center; width: 15px; } -.review-demo__nav-footer { border-top: 1px solid var(--review-border); color: var(--review-muted); display: flex; font-size: 9px; justify-content: space-between; padding: 9px 12px; } - -.review-demo__canvas { min-height: 0; min-width: 0; overflow: hidden; } -.review-demo__diff-scroll { height: 100%; min-width: 0; overflow: auto; padding: 14px; } - -.review-demo__summary { border-bottom: 1px solid var(--review-border); padding: 16px 14px; } -.review-demo__summary-heading { align-items: center; display: flex; gap: 9px; } -.review-demo__summary-heading > div { display: flex; flex-direction: column; } -.review-demo__summary-heading strong { font-size: 12px; } -.review-demo__summary-heading span { color: var(--review-muted); font-size: 9px; } -.review-demo__summary > p { color: var(--review-muted); font-size: 11px; line-height: 1.5; margin: 12px 0; } -.review-demo__checks { display: flex; flex-direction: column; gap: 5px; } -.review-demo__checks span { align-items: center; color: var(--review-muted); display: flex; font-size: 10px; gap: 6px; } -.review-demo__checks span:first-child { color: #218354; } - -.review-demo__comment-section { padding: 13px 10px 28px; } -.review-demo__section-heading { align-items: center; display: flex; justify-content: space-between; margin: 0 4px 10px; } -.review-demo__section-heading strong { font-size: 11px; text-transform: uppercase; letter-spacing: .05em; } -.review-demo__section-heading span { background: var(--review-subtle); border-radius: 5px; color: var(--review-muted); font: 10px ui-monospace, monospace; padding: 2px 5px; } -.review-demo__comment-empty { align-items: center; border: 1px dashed var(--review-border-strong); border-radius: 8px; color: var(--review-muted); display: flex; flex-direction: column; padding: 18px 13px; text-align: center; } -.review-demo__comment-empty strong { color: var(--review-text); font-size: 11px; margin-top: 7px; } -.review-demo__comment-empty p { font-size: 10px; line-height: 1.45; margin: 4px 0 0; } - -.review-demo__composer { border: 1px solid var(--review-border-strong); border-radius: 8px; margin-bottom: 10px; overflow: hidden; } -.review-demo__composer-heading { align-items: center; background: var(--review-subtle); border-bottom: 1px solid var(--review-border); display: flex; font-size: 10px; justify-content: space-between; padding: 7px 9px; } -.review-demo__composer-heading span { color: var(--review-muted); } -.review-demo__composer textarea { background: var(--review-surface); border: 0; color: var(--review-text); min-height: 88px; outline: none; padding: 9px; resize: vertical; width: 100%; } -.review-demo__composer textarea::placeholder { color: var(--review-muted); } -.review-demo__composer-actions { align-items: center; border-top: 1px solid var(--review-border); display: flex; justify-content: flex-end; gap: 5px; padding: 6px; } - -.review-demo__thread { border: 1px solid var(--review-border); border-radius: 8px; margin-top: 10px; padding: 10px; } -.review-demo__thread[data-resolved="true"] { opacity: .62; } -.review-demo__thread-heading { align-items: center; display: grid; gap: 7px; grid-template-columns: 25px minmax(0, 1fr) auto; } -.review-demo__avatar { align-items: center; background: color-mix(in srgb, #4f7bef 15%, var(--review-surface)); border-radius: 50%; color: #4f6fc1; display: flex; font-size: 8px; font-weight: 750; height: 25px; justify-content: center; width: 25px; } -.review-demo__thread-heading > div { display: flex; flex-direction: column; } -.review-demo__thread-heading strong { font-size: 10px; } -.review-demo__thread-heading div span { color: var(--review-muted); font-size: 9px; } -.review-demo__thread-status { color: #a27504; font-size: 8px; font-weight: 700; text-transform: uppercase; } -.review-demo__thread[data-resolved="true"] .review-demo__thread-status { color: #218354; } -.review-demo__thread p { font-size: 10px; line-height: 1.5; margin: 9px 0; } -.review-demo__resolve { background: transparent; border: 0; color: var(--review-muted); cursor: pointer; font-size: 9px; font-weight: 650; padding: 0; } -.review-demo__resolve:hover { color: var(--review-accent); } - -@media (max-width: 1120px) { - .review-demo__workspace { grid-template-columns: 210px minmax(480px, 1fr); } - .review-demo__inspector { display: none; } -} - -@media (max-width: 760px) { - .review-demo__workspace { grid-template-columns: minmax(0, 1fr); } - .review-demo__navigation { display: none; } - .review-demo__crumbs span, .review-demo__crumbs svg { display: none; } - .review-demo__diff-scroll { padding: 8px; } + border: 0; + border-radius: 0; + overflow-x: auto; +} +.cr-demo .fk-diff-viewer__line { + min-width: max-content; +} +.cr-demo .fk-diff-viewer__widget { + min-width: 0; + max-width: 100%; +} +.cr-demo .fk-review__thread .fk-diff-viewer { + border: 1px solid var(--border); + border-radius: 6px; +} +.cr-demo .fk-review__panel { + margin: 0 0 16px; + padding: 14px; + background: var(--card); +} +.cr-demo .fk-review__panel input[type="search"] { + width: 100%; + margin-block: 6px; +} +.cr-demo .fk-review__panel > label { + display: block; + margin-block: 6px; +} +.cr-demo .fk-review__panel ul { + list-style: none; + padding: 0; + margin: 0; +} +.cr-demo .fk-review__panel li { + padding-block: 6px; +} +.cr-demo .fk-review__panel p { + color: var(--muted-foreground); + margin-block: 6px; +} +.cr-demo .fk-review__panel textarea { + min-height: 70px; +} +.cr-demo .fk-review__thread { + background: var(--card); +} +.cr-demo .fk-review__comment time { + display: block; + margin: 0; + color: var(--muted-foreground); +} +.cr-demo .fk-review__list [role="option"] { + padding: 8px; + margin-block: 4px; + border-radius: 4px; + overflow-wrap: anywhere; + cursor: pointer; +} +.cr-demo .fk-review__list h4 { + font-size: 11px; + overflow-wrap: anywhere; +} +.cr-demo .fk-review__thread[data-anchor-state="outdated"] { + border-style: dashed; +} +.cr-demo .fk-review__thread[data-anchor-state="moved"] { + border-color: #39a86b; +} +.cr-demo__empty { + padding: 24px; + color: var(--muted-foreground); +} +.cr-demo__announcement { + padding: 12px 0; + color: var(--muted-foreground); +} +@media (max-width: 1280px) { + .cr-demo__workspace { + grid-template-columns: 200px minmax(0, 1fr); + } + .cr-demo__inspector { + grid-column: 1 / -1; + display: grid; + grid-template-columns: repeat(2, minmax(0, 1fr)); + gap: 16px; + } +} +@media (max-width: 700px) { + .cr-demo { + padding: 12px; + } + .cr-demo__workspace, + .cr-demo__inspector { + display: flex; + flex-direction: column; + gap: 16px; + } + .cr-demo__navigation, + .cr-demo__canvas, + .cr-demo__inspector, + .cr-demo__inspector > * { + width: 100%; + } + .cr-demo__actor { + margin-inline-start: 0; + } + .cr-demo .fk-diff-viewer__split-row { + min-width: 580px; + } + .cr-demo .fk-diff-viewer__header { + position: static; + } + .cr-demo .fk-review__header header { + display: flex; + flex-wrap: wrap; + } + .cr-demo .fk-review__header header > * { + min-width: 0; + } } diff --git a/apps/demo/src/diff-viewer/update.test.ts b/apps/demo/src/diff-viewer/update.test.ts index 6974269..971b382 100644 --- a/apps/demo/src/diff-viewer/update.test.ts +++ b/apps/demo/src/diff-viewer/update.test.ts @@ -1,45 +1,181 @@ import { describe, expect, it } from "vitest"; - +import { ReviewThreads } from "@foldworks/review"; +import { CodeEditor } from "@foldworks/code-editor"; import { Message } from "./message"; -import { initialModel, type Model } from "./model"; +import { freshModel, type Model } from "./model"; +import { proposedFlags, suggestedFlags, editorFor } from "./fixture"; import { update } from "./update"; - -const reduce = (model: Model, message: Message): Model => update(model, message).model; - -describe("diff review selection", () => { - it("selects a pointer range and stores normalized comment endpoints", () => { - const path = initialModel.activePath; - let model = reduce(initialModel, Message.StartedSelection({ path, side: "new", line: 24 })); - model = reduce(model, Message.ExtendedSelection({ path, side: "new", line: 21, method: "Pointer" })); - model = reduce(model, Message.EndedSelection()); - - expect(model).toMatchObject({ - selectionStartLine: 24, - selectionEndLine: 21, - selectionDragging: false, - }); - - model = reduce(model, Message.ChangedDraft({ value: "Cover this whole branch." })); - model = reduce(model, Message.SubmittedComment()); - - expect(model.comments.at(-1)).toMatchObject({ - path, +import { checks, mappedThread, canMerge } from "./domain"; +const reduce = (model: Model, message: Message) => update(model, message).model; +const child = (model: Model, message: ReviewThreads.Message) => + reduce(model, Message.GotThreadsMessage({ message })); +const proposal = (): Model => { + const model = freshModel(), + file = model.files[0]!; + return { + ...model, + files: model.files.map((f) => (f.path === file.path ? { ...f, text: proposedFlags } : f)), + editor: editorFor(file.path).init({ + id: model.editor.id, + uri: file.path, + languageId: "json", + text: proposedFlags, + }), + }; +}; +const comment = ( + model: Model, + start: number, + end: number, + body: string, + suggestion = false, +): Model => { + model = reduce( + model, + Message.StartedSelection({ path: model.activePath, side: "new", line: start }), + ); + model = reduce( + model, + Message.ExtendedSelection({ + path: model.activePath, side: "new", - startLine: 21, - endLine: 24, - body: "Cover this whole branch.", - resolved: false, - }); - expect(model.selectionPath).toBe(""); + line: end, + method: "Keyboard", + }), + ); + model = reduce(model, Message.EndedSelection()); + const draftKey = model.reviewUi.composerKey; + model = child(model, ReviewThreads.Message.ChangedDraft({ draftKey, value: body })); + const expected = proposedFlags + .trimEnd() + .split("\n") + .slice(start - 1, end) + .join("\n"); + if (suggestion) { + model = child( + model, + ReviewThreads.Message.ToggledSuggestion({ draftKey, enabled: true, value: expected }), + ); + model = child( + model, + ReviewThreads.Message.ChangedSuggestion({ draftKey, value: suggestedFlags }), + ); + } + return child( + model, + ReviewThreads.Message.Submitted({ + draftKey, + kind: "create", + targetId: draftKey, + ...(suggestion + ? { + suggestionAnchor: { + path: model.activePath, + startLine: start, + endLine: end, + expected, + }, + } + : {}), + }), + ); +}; +describe("local change request", () => { + it("completes pending review, suggestion, anchor remapping, current approval and merge history", () => { + let model = reduce(proposal(), Message.OpenedRequest()); + expect(checks(model).map((c) => c.status)).toEqual(["passed", "failed"]); + model = reduce(model, Message.SwitchedActor({ actor: "reviewer" })); + model = comment(model, 4, 4, "Remove this temporary note."); + model = comment(model, 3, 4, "Use a smaller rollout.", true); + expect(model.threads.every((t) => t.comments[0]!.pending)).toBe(true); + expect(model.reviewUi.drafts).toEqual({}); + model = reduce(model, Message.ChangedVerdict({ verdict: "request-changes" })); + model = reduce(model, Message.SubmittedReview()); + model = reduce(model, Message.SwitchedActor({ actor: "author" })); + const thread = model.threads[1]!; + model = reduce( + model, + Message.AppliedSuggestion({ + threadId: thread.id, + suggestion: thread.comments[0]!.suggestion!, + }), + ); + expect(mappedThread(model, model.threads[0]!).state).toBe("outdated"); + expect(mappedThread(model, model.threads[1]!).state).toBe("moved"); + expect(model.threads[1]!.status).toBe("resolved"); + expect(checks(model).every((c) => c.status === "passed")).toBe(true); + expect(canMerge(model)).toBe(false); + model = reduce(model, Message.SwitchedActor({ actor: "reviewer" })); + model = reduce(model, Message.ChangedVerdict({ verdict: "approve" })); + model = reduce(model, Message.SubmittedReview()); + model = reduce(model, Message.SwitchedActor({ actor: "author" })); + expect(canMerge(model)).toBe(true); + const merged = reduce(model, Message.Merged()); + expect(merged.status).toBe("merged"); + expect(merged.files.every((f) => f.text === f.baseText)).toBe(true); + expect(merged.activity.map((e) => e.kind)).toContain("suggestion"); + expect(merged.activity.at(-1)!.title).toBe("Configuration merged locally"); + const stale = { + ...model, + files: model.files.map((f, i) => + i === 0 ? { ...f, text: f.text.replace('"rollout": 25', '"rollout": 20') } : f, + ), + }; + expect(canMerge(stale)).toBe(false); }); - - it("extends a keyboard selection but ignores pointer hover outside a drag", () => { - const path = initialModel.activePath; - let model = reduce(initialModel, Message.SelectedLine({ path, side: "new", line: 20 })); - model = reduce(model, Message.ExtendedSelection({ path, side: "new", line: 21, method: "Pointer" })); - expect(model.selectionEndLine).toBe(20); - - model = reduce(model, Message.ExtendedSelection({ path, side: "new", line: 21, method: "Keyboard" })); - expect(model).toMatchObject({ selectionStartLine: 20, selectionEndLine: 21 }); + it("blocks unauthorized merge/application and preserves text on stale suggestions", () => { + let model = reduce(proposal(), Message.OpenedRequest()); + model = reduce(model, Message.SwitchedActor({ actor: "reviewer" })); + model = comment(model, 3, 4, "Suggestion", true); + model = reduce(model, Message.SubmittedReview()); + const suggestion = model.threads[0]!.comments[0]!.suggestion!; + expect( + reduce(model, Message.AppliedSuggestion({ suggestion, threadId: model.threads[0]!.id })) + .files, + ).toEqual(model.files); + model = reduce(model, Message.SwitchedActor({ actor: "author" })); + model = { + ...model, + files: model.files.map((f, i) => + i === 0 ? { ...f, text: f.text.replace('"rollout": 100', '"rollout": 99') } : f, + ), + }; + const next = reduce( + model, + Message.AppliedSuggestion({ suggestion, threadId: model.threads[0]!.id }), + ); + expect(next.files).toEqual(model.files); + expect(next.announcement).toContain("stale"); + expect(reduce(next, Message.Merged()).status).toBe("open"); + }); + it("validates editor messages and retains draft data when persistence fails", () => { + let model = freshModel(); + model = reduce( + model, + Message.GotEditorMessage({ + message: CodeEditor.execute( + CodeEditor.Operation.ReplaceDocument({ + uri: model.activePath, + text: '{"checkout":"yes","rollout":0}', + languageId: "json", + }), + ), + }), + ); + expect(checks(model)[0]!.status).toBe("failed"); + model = child( + model, + ReviewThreads.Message.ChangedDraft({ draftKey: "anchor", value: "Keep me" }), + ); + expect( + reduce(model, Message.Persisted({ error: "Storage unavailable" })).reviewUi.drafts.anchor, + ).toBe("Keep me"); }); }); + +it("advances the editor session when switching files to reject delayed native events", () => { + const initial = freshModel(); + const next = reduce(initial, Message.SelectedFile({ path: "config/access.json" })); + expect(next.editor.document.session).toBeGreaterThan(initial.editor.document.session); + expect(next.editor.document.uri).toBe("config/access.json"); +}); diff --git a/apps/demo/src/diff-viewer/update.ts b/apps/demo/src/diff-viewer/update.ts index 7f2c009..ec3bf33 100644 --- a/apps/demo/src/diff-viewer/update.ts +++ b/apps/demo/src/diff-viewer/update.ts @@ -1,116 +1,495 @@ -import type { Update } from "foldkit"; - +import { Effect, Option, Schema as S } from "effect"; +import { Command, Update } from "foldkit"; +import * as Dom from "foldkit/dom"; +import { CodeEditor } from "@foldworks/code-editor"; +import { ReviewThreads, ChangedFiles, applySuggestions, type Comment } from "@foldworks/review"; +import { lineGutterId, normalizeSelection } from "@foldworks/diff-viewer"; import { Message } from "./message"; -import type { Model } from "./model"; +import { freshModel, type Model, type Anchor } from "./model"; +import { author, reviewer, editorFor, storageKey } from "./fixture"; +import { fileDiff, changedFiles, revisionKey, checks, canMerge, mappedThread } from "./domain"; -export const update = (model: Model, message: Message): Update.Return => - Message.match(message, { - SelectedFile: ({ path }) => ({ - model: { +const Persist = Command.define("PersistLocalChangeRequest", { + args: { snapshot: S.String }, + messages: [Message.Persisted], + execute: ({ snapshot }) => + Effect.sync(() => { + try { + localStorage.setItem(storageKey, snapshot); + return Message.Persisted({ error: "" }); + } catch { + return Message.Persisted({ + error: "Browser storage failed. Your changes remain in this session.", + }); + } + }), +}); +const Reveal = Command.define("RevealReviewAnchor", { + args: { id: S.String }, + messages: [Message.CompletedReveal], + execute: ({ id }) => + Dom.focus(`#${CSS.escape(id)}`).pipe( + Effect.andThen(Dom.scrollIntoView(`#${CSS.escape(id)}`, { block: "center" })), + Effect.ignore, + Effect.as(Message.CompletedReveal()), + ), +}); +const actor = (model: Model) => (model.actor === "author" ? author : reviewer); +const event = (model: Model, title: string, kind: string, context = ""): Model => ({ + ...model, + activity: [ + ...model.activity, + { + id: `event-${model.activity.length + 1}`, + title, + kind, + actor: actor(model).name, + timestamp: new Date().toISOString(), + context, + changes: [], + }, + ], + announcement: title, +}); +const selectFile = (model: Model, path: string): Model => { + const file = model.files.find((f) => f.path === path); + return !file + ? model + : { ...model, activePath: path, - selectionPath: "", - selectionStartLine: 0, - selectionEndLine: 0, - selectionDragging: false, - draft: "", - announcement: `Opened ${path}.`, + selection: null, + dragging: false, + editor: { + ...editorFor(path).update( + model.editor, + CodeEditor.execute( + CodeEditor.Operation.ReplaceDocument({ + uri: path, + text: file.text, + languageId: "json", + }), + ), + ).model, + options: { + ...model.editor.options, + theme: model.dark ? "dark" : "light", + readOnly: model.actor !== "author" || model.status === "merged", + }, + }, + }; +}; +const foldEditor = (parent: Model, message: CodeEditor.Message) => + Update.foldChild({ + update: editorFor(parent.activePath).update, + read: (model: Model) => Option.some(model.editor), + write: (model, editor) => ({ ...model, editor }), + toParentMessage: (message) => Message.GotEditorMessage({ message }), + foldOutMessage: + (out) => + (model: Model): Update.Return => { + if (out._tag !== "ChangedDocument") return { model }; + if (model.actor !== "author" || model.status === "merged") + return { model: selectFile(model, model.activePath) }; + return { + model: { + ...model, + files: model.files.map((f) => + f.path === model.activePath ? { ...f, text: out.document.text } : f, + ), + viewedPaths: model.viewedPaths.filter((p) => p !== model.activePath), + expandedGaps: [], + announcement: "Working configuration updated. Local checks recalculated.", + }, + }; }, + })(parent, message); +const foldFiles = (parent: Model, message: ChangedFiles.Message) => + Update.foldChild({ + update: (model: ChangedFiles.Model, message: ChangedFiles.Message) => + ChangedFiles.update(model, message, { files: changedFiles(parent) }), + read: (model: Model) => Option.some(model.filesUi), + write: (model, filesUi) => ({ ...model, filesUi }), + toParentMessage: (message) => Message.GotFilesMessage({ message }), + foldOutMessage: + (out) => + (model: Model): Update.Return => + out._tag === "SelectedFile" + ? { model: selectFile(model, out.path) } + : { + model: { + ...model, + viewedPaths: out.viewed + ? [...new Set([...model.viewedPaths, out.path])] + : model.viewedPaths.filter((p) => p !== out.path), + }, + }, + })(parent, message); +const foldThreads = Update.foldChild({ + update: ReviewThreads.update, + read: (model: Model) => Option.some(model.reviewUi), + write: (model, reviewUi) => ({ ...model, reviewUi }), + toParentMessage: (message) => Message.GotThreadsMessage({ message }), + foldOutMessage: + (out) => + (model: Model): Update.Return => { + if (out._tag === "RequestedActivate") { + const thread = model.threads.find((t) => t.id === out.threadId); + if (!thread) return { model }; + const mapping = mappedThread(model, thread); + const next = selectFile(model, thread.anchor.path); + return { + model: { + ...next, + semantic: false, + reviewUi: { ...next.reviewUi, activeThreadId: thread.id }, + }, + commands: [ + Reveal({ + id: + mapping.state === "outdated" || + !fileDiff(model.files.find((f) => f.path === thread.anchor.path)!).hunks.length + ? `review-thread-${thread.id}` + : lineGutterId({ + path: mapping.range.path, + side: mapping.range.side, + line: mapping.range.endLine, + }), + }), + ], + }; + } + if (model.status !== "open") return { model }; + if (out._tag === "RequestedResolve" || out._tag === "RequestedReopen") + return { + model: event( + { + ...model, + threads: model.threads.map((t) => + t.id === out.threadId + ? { ...t, status: out._tag === "RequestedResolve" ? "resolved" : "open" } + : t, + ), + }, + out._tag === "RequestedResolve" ? "Thread resolved" : "Thread reopened", + "comment", + ), + }; + if (out._tag === "RequestedDelete") + return { + model: { + ...model, + threads: model.threads + .map((t) => + t.id === out.threadId + ? { + ...t, + comments: t.comments.filter( + (c) => c.id !== out.commentId || c.author.id !== model.actor, + ), + } + : t, + ) + .filter((t) => t.comments.length), + }, + }; + const acknowledge = (next: Model) => + foldThreads( + next, + ReviewThreads.Message.Committed({ + draftKey: out.draftKey, + body: out.body, + ...("suggestion" in out && out.suggestion ? { suggestion: out.suggestion } : {}), + }), + ); + if (out._tag === "RequestedEdit") { + const own = model.threads.some((t) => + t.comments.some((c) => c.id === out.commentId && c.author.id === model.actor), + ); + if (!own) return { model }; + return acknowledge({ + ...model, + threads: model.threads.map((t) => ({ + ...t, + comments: t.comments.map((c) => + c.id === out.commentId + ? { ...c, body: out.body, editedAt: new Date().toISOString() } + : c, + ), + })), + }); + } + const comment: Comment = { + id: `comment-${model.nextId}`, + author: actor(model), + body: out.body, + createdAt: new Date().toISOString(), + pending: model.actor === "reviewer", + ...(out.suggestion ? { suggestion: out.suggestion } : {}), + }; + if (out._tag === "RequestedReply") { + if (!model.threads.some((t) => t.id === out.threadId)) return { model }; + return acknowledge( + event( + { + ...model, + nextId: model.nextId + 1, + threads: model.threads.map((t) => + t.id === out.threadId ? { ...t, comments: [...t.comments, comment] } : t, + ), + }, + "Reply saved locally", + "comment", + ), + ); + } + const anchor = model.draftAnchors[out.anchorKey]; + if (!anchor || !comment.body.trim()) return { model }; + if ( + comment.suggestion && + (anchor.side !== "new" || + comment.suggestion.path !== anchor.path || + comment.suggestion.startLine !== anchor.startLine || + comment.suggestion.endLine !== anchor.endLine) + ) + return { model }; + return acknowledge( + event( + { + ...model, + nextId: model.nextId + 1, + selection: null, + dragging: false, + threads: [ + ...model.threads, + { id: `thread-${model.nextId}`, anchor, status: "open", comments: [comment] }, + ], + }, + comment.pending ? "Pending comment saved locally" : "Comment saved locally", + "comment", + ), + ); + }, +}); +const openComposer = (model: Model): Update.Return => { + if (!model.selection || model.status !== "open") return { model }; + const range = normalizeSelection(model.selection), + file = model.files.find((f) => f.path === range.path); + if (!file) return { model }; + const text = range.side === "old" ? file.baseText : file.text; + const key = `new:${range.path}:${range.side}:${range.startLine}:${range.endLine}:${text}`; + const anchor: Anchor = { ...range, revisionText: text, originalText: text }; + return foldThreads( + { ...model, draftAnchors: { ...model.draftAnchors, [key]: anchor } }, + ReviewThreads.Message.OpenedComposer({ + draftKey: key, + returnFocusId: lineGutterId({ + path: range.path, + side: range.side, + line: model.selection.endLine, + }), }), - ChangedMode: ({ mode }) => ({ model: { ...model, mode, announcement: `${mode} diff selected.` } }), - SelectedLine: ({ path, side, line }) => ({ + ); +}; +const core = (model: Model, message: Message): Update.Return => + Message.match>(message, { + GotEditorMessage: ({ message }) => foldEditor(model, message), + GotThreadsMessage: ({ message }) => + foldThreads( + message._tag === "Cancelled" ? { ...model, selection: null, dragging: false } : model, + message, + ), + GotFilesMessage: ({ message }) => foldFiles(model, message), + SelectedFile: ({ path }) => ({ model: selectFile(model, path) }), + ChangedMode: ({ mode }) => ({ model: { ...model, mode } }), + ToggledSemantic: () => ({ model: { ...model, semantic: !model.semantic } }), + ToggledDark: () => ({ model: { ...model, - selectionPath: path, - selectionSide: side, - selectionStartLine: line, - selectionEndLine: line, - selectionDragging: false, - draft: "", - announcement: `Selected ${side} line ${line} in ${path}.`, + dark: !model.dark, + editor: { + ...model.editor, + options: { ...model.editor.options, theme: model.dark ? "light" : "dark" }, + }, }, }), + SwitchedActor: ({ actor }) => ({ + model: selectFile( + { ...model, actor, selection: null, reviewUi: { ...model.reviewUi, composerKey: "" } }, + model.activePath, + ), + }), + SelectedLine: ({ path, side, line }) => + openComposer({ + ...model, + selection: { path, side, startLine: line, endLine: line }, + dragging: false, + }), StartedSelection: ({ path, side, line }) => ({ model: { ...model, - selectionPath: path, - selectionSide: side, - selectionStartLine: line, - selectionEndLine: line, - selectionDragging: true, - draft: "", - announcement: `Started selection at ${side} line ${line}.`, + selection: { path, side, startLine: line, endLine: line }, + dragging: true, }, }), ExtendedSelection: ({ path, side, line, method }) => { - const sameAnchor = model.selectionPath === path && model.selectionSide === side; - if (!sameAnchor || (method === "Pointer" && !model.selectionDragging)) return { model }; - const start = Math.min(model.selectionStartLine, line); - const end = Math.max(model.selectionStartLine, line); - return { - model: { - ...model, - selectionEndLine: line, - announcement: `Selected ${side} lines ${start} through ${end}.`, - }, - }; + if ( + !model.selection || + model.selection.path !== path || + model.selection.side !== side || + (method === "Pointer" && !model.dragging) + ) + return { model }; + const next = { ...model, selection: { ...model.selection, endLine: line } }; + return method === "Keyboard" ? openComposer(next) : { model: next }; }, - EndedSelection: () => !model.selectionDragging - ? ({ model }) - : ({ model: { ...model, selectionDragging: false } }), - ChangedDraft: ({ value }) => ({ model: { ...model, draft: value } }), - SubmittedComment: () => { - const body = model.draft.trim(); - if (body === "" || model.selectionPath === "" || model.selectionStartLine < 1) return { model }; - const startLine = Math.min(model.selectionStartLine, model.selectionEndLine); - const endLine = Math.max(model.selectionStartLine, model.selectionEndLine); - return { - model: { - ...model, - comments: [...model.comments, { - id: `comment-${model.comments.length + 1}`, - path: model.selectionPath, - side: model.selectionSide, - startLine, - endLine, - author: "You", - body, - resolved: false, - }], - selectionPath: "", - selectionStartLine: 0, - selectionEndLine: 0, - selectionDragging: false, - draft: "", - announcement: "Review comment added.", - }, - }; - }, - CancelledComment: () => ({ - model: { - ...model, - selectionPath: "", - selectionStartLine: 0, - selectionEndLine: 0, - selectionDragging: false, - draft: "", - announcement: "Comment cancelled.", - }, - }), - ToggledViewed: ({ path, viewed }) => ({ + EndedSelection: () => + model.dragging ? openComposer({ ...model, dragging: false }) : { model }, + CancelledSelection: () => + foldThreads( + { ...model, selection: null, dragging: false }, + ReviewThreads.Message.Cancelled(), + ), + ExpandedGap: ({ id }) => ({ model: { ...model, expandedGaps: [...model.expandedGaps, id] } }), + ChangedViewed: ({ path, viewed }) => ({ model: { ...model, viewedPaths: viewed ? [...new Set([...model.viewedPaths, path])] - : model.viewedPaths.filter((item) => item !== path), - announcement: `${path} marked ${viewed ? "viewed" : "unviewed"}.`, + : model.viewedPaths.filter((p) => p !== path), }, }), - ToggledResolved: ({ id }) => ({ - model: { - ...model, - comments: model.comments.map((comment) => comment.id === id ? { ...comment, resolved: !comment.resolved } : comment), - announcement: "Review thread updated.", - }, + OpenedRequest: () => + model.actor !== "author" || + model.status !== "draft" || + !changedFiles(model).length || + checks(model)[0]!.status !== "passed" + ? { model } + : { + model: event( + { ...model, status: "open" }, + "Change request opened", + "status", + "Local configuration proposal, stored in this browser.", + ), + }, + AppliedSuggestion: ({ suggestion, threadId }) => { + if (model.actor !== "author" || model.status !== "open") return { model }; + const thread = model.threads.find((t) => t.id === threadId), + file = model.files.find((f) => f.path === suggestion.path); + if ( + !thread || + !file || + !thread.comments.some( + (c) => + !c.pending && + c.suggestion && + JSON.stringify(c.suggestion) === JSON.stringify(suggestion), + ) + ) + return { model }; + const result = applySuggestions(file.text, [suggestion]); + if (result.status !== "applied") + return { + model: { + ...model, + announcement: `Suggestion rejected: ${result.reason}. Draft and threads retained.`, + }, + }; + const replacementLines = + suggestion.replacement === "" + ? 0 + : suggestion.replacement.replace(/\n$/, "").split("\n").length; + const next = event( + { + ...model, + files: model.files.map((f) => (f.path === file.path ? { ...f, text: result.text } : f)), + threads: model.threads.map((t) => + t.id === threadId + ? { + ...t, + status: "resolved", + anchor: { + ...t.anchor, + side: "new", + revisionText: result.text, + startLine: suggestion.startLine, + endLine: suggestion.startLine + Math.max(1, replacementLines) - 1, + }, + } + : t, + ), + viewedPaths: model.viewedPaths.filter((p) => p !== file.path), + expandedGaps: [], + }, + "Suggestion applied locally", + "suggestion", + "Host committed replacement text and resolved its thread.", + ); + return { model: selectFile(next, file.path) }; + }, + ChangedSummary: ({ value }) => ({ model: { ...model, summary: value } }), + ChangedVerdict: ({ verdict }) => ({ model: { ...model, verdict } }), + SubmittedReview: () => + model.actor !== "reviewer" || model.status !== "open" + ? { model } + : { + model: event( + { + ...model, + reviews: [ + ...model.reviews, + { verdict: model.verdict, summary: model.summary, revision: revisionKey(model) }, + ], + threads: model.threads.map((t) => ({ + ...t, + comments: t.comments.map((c) => + c.author.id === model.actor ? { ...c, pending: false } : c, + ), + })), + summary: "", + }, + `Review submitted: ${model.verdict}`, + "review", + model.summary, + ), + }, + ReranChecks: () => ({ + model: event( + model, + "Local checks recalculated", + "check", + checks(model) + .map((c) => `${c.name}: ${c.status}`) + .join("; "), + ), }), + Merged: () => + !canMerge(model) + ? { model: { ...model, announcement: "Merge requirements are not met." } } + : { + model: event( + { + ...model, + status: "merged", + files: model.files.map((f) => ({ ...f, baseText: f.text })), + editor: { ...model.editor, options: { ...model.editor.options, readOnly: true } }, + }, + "Configuration merged locally", + "status", + "Saved configuration now contains the reviewed text. No external repository is connected.", + ), + }, + Reset: () => ({ model: freshModel() }), + Persisted: ({ error }) => ({ model: { ...model, storageError: error } }), + CompletedReveal: () => ({ model }), }); +export const update = (model: Model, message: Message): Update.Return => { + const result = core(model, message); + return message._tag === "Persisted" || message._tag === "CompletedReveal" + ? result + : { + ...result, + commands: [...(result.commands ?? []), Persist({ snapshot: JSON.stringify(result.model) })], + }; +}; diff --git a/apps/demo/src/diff-viewer/view.ts b/apps/demo/src/diff-viewer/view.ts index 9dc6e52..2bad11f 100644 --- a/apps/demo/src/diff-viewer/view.ts +++ b/apps/demo/src/diff-viewer/view.ts @@ -1,204 +1,528 @@ -import { Check, ChevronRight, CircleCheck, FileCode2, GitPullRequest, MessageSquare, SplitSquareHorizontal, Rows3 } from "@lucide/icons"; -import { DiffViewer, diffTotals, selectionLabel, type DiffFile, type DiffSelectionRange, type DiffThreadMarker } from "@foldworks/diff-viewer"; -import { Button, Icon } from "@foldworks/ui"; +import { + DiffViewer, + diffFile, + ValueDiff, + diffValues, + diagnosticsToAnnotations, + contextGaps, + expandContext, + normalizeSelection, + selectionLabel, + type DiffLineRef, +} from "@foldworks/diff-viewer"; +import { CodeEditor } from "@foldworks/code-editor"; +import { + ChangedFiles, + CheckList, + MergeReadiness, + ChangeRequestHeader, + ReviewThreads, + ReviewSubmit, + ReviewerList, +} from "@foldworks/review"; +import { TransactionTimeline, ChangeSetPreview } from "@foldworks/ui"; import type { Html, HtmlBuilder } from "foldkit/html"; import { defineView } from "foldkit/submodel"; - -import { reviewFiles } from "./fixture"; +import type { Model, ReviewThread } from "./model"; import { Message } from "./message"; -import type { Model, ReviewComment } from "./model"; - -const statusLabel = (file: DiffFile): string => - file.status === "added" ? "Added" : file.status === "deleted" ? "Deleted" : file.status === "renamed" ? "Renamed" : "Modified"; +import { author, reviewer } from "./fixture"; +import { + currentFile, + fileDiff, + changedFiles, + diagnostics, + checks, + mappedThread, + approved, + openCurrentThreads, + canMerge, + parseValue, + reviewHighlight, +} from "./domain"; -const fileItem = (model: Model, file: DiffFile, h: HtmlBuilder): Html => { - const viewed = model.viewedPaths.includes(file.path); - const commentCount = model.comments.filter((comment) => comment.path === file.path && !comment.resolved).length; - return h.button([ - h.Type("button"), - h.Class("review-demo__file"), - h.DataAttribute("active", model.activePath === file.path ? "true" : "false"), - h.OnClick(Message.SelectedFile({ path: file.path })), - h.AriaLabel(`${file.path}, ${statusLabel(file)}, ${file.additions} additions, ${file.deletions} deletions${viewed ? ", viewed" : ""}`), - ], [ - h.span([h.Class("review-demo__file-state"), h.DataAttribute("status", file.status)], [ - viewed ? Icon.view({ icon: Check, size: 12, strokeWidth: 2.6 }, h) : statusLabel(file).slice(0, 1), - ]), - h.span([h.Class("review-demo__file-copy")], [ - h.strong([], [file.path.split("/").pop() ?? file.path]), - h.span([], [file.path.includes("/") ? file.path.slice(0, file.path.lastIndexOf("/")) : "Repository root"]), - ]), - h.span([h.Class("review-demo__file-stats")], [ - h.span([h.Class("review-demo__plus")], [`+${file.additions}`]), - h.span([h.Class("review-demo__minus")], [`−${file.deletions}`]), - ...(commentCount === 0 ? [] : [h.span([h.Class("review-demo__comment-count")], [String(commentCount)])]), - ]), - ]); +const button = (label: string, message: Message, h: HtmlBuilder, disabled = false) => + h.button( + [h.Type("button"), h.Class("cr-demo__button"), h.OnClick(message), h.Disabled(disabled)], + [label], + ); +const threadView = (model: Model, thread: ReviewThread, h: HtmlBuilder): Html => { + const mapping = mappedThread(model, thread); + return h.div( + [h.Id(`review-thread-${thread.id}`), h.Tabindex(-1)], + [ + ReviewThreads.threadView( + { + model: model.reviewUi, + thread, + readOnly: model.status !== "open", + anchorState: mapping.state, + label: `${selectionLabel(mapping.state === "outdated" ? thread.anchor : mapping.range)} · ${thread.anchor.path.split("/").at(-1)}`, + toMessage: (message) => Message.GotThreadsMessage({ message }), + canEdit: (c) => model.status === "open" && c.author.id === model.actor, + canDelete: (c) => model.status === "open" && c.author.id === model.actor, + ...(model.actor === "author" && + model.status === "open" && + thread.status === "open" && + !thread.comments.some((c) => c.pending) + ? { + onApplySuggestion: ( + suggestion: import("@foldworks/review").Suggestion, + threadId: string, + ) => Message.AppliedSuggestion({ suggestion, threadId }), + } + : {}), + }, + h, + ), + ], + ); }; - -const navigation = (model: Model, h: HtmlBuilder): Html => { - const totals = diffTotals(reviewFiles); - return h.aside([h.Class("review-demo__navigation"), h.AriaLabel("Changed files")], [ - h.div([h.Class("review-demo__nav-heading")], [ - h.div([], [h.strong([], ["Files changed"]), h.span([], [`${reviewFiles.length}`])]), - h.p([], [`${totals.additions} additions and ${totals.deletions} deletions`]), - ]), - h.div([h.Class("review-demo__progress-track"), h.AriaLabel(`${model.viewedPaths.length} of ${reviewFiles.length} files viewed`)], [ - h.span([h.Style({ width: `${(model.viewedPaths.length / reviewFiles.length) * 100}%` })], []), - ]), - h.div([h.Class("review-demo__file-list")], reviewFiles.map((file) => fileItem(model, file, h))), - h.div([h.Class("review-demo__nav-footer")], [ - h.span([], [`${model.viewedPaths.length} of ${reviewFiles.length} viewed`]), - h.span([], ["⌘⇧Enter to submit"]), - ]), - ]); +const lineWidgets = (model: Model, ref: DiffLineRef, h: HtmlBuilder): Html | undefined => { + const items = model.threads.flatMap((thread) => { + const mapping = mappedThread(model, thread); + return mapping.state !== "outdated" && + mapping.range.path === ref.path && + mapping.range.side === ref.side && + mapping.range.endLine === ref.line + ? [threadView(model, thread, h)] + : []; + }); + const key = model.reviewUi.composerKey, + anchor = model.draftAnchors[key]; + if ( + key.startsWith("new:") && + anchor && + anchor.path === ref.path && + anchor.side === ref.side && + anchor.endLine === ref.line + ) + items.push( + h.strong([], [selectionLabel(anchor)]), + ReviewThreads.composerView( + { + model: model.reviewUi, + draftKey: key, + targetId: key, + kind: "create", + toMessage: (message) => Message.GotThreadsMessage({ message }), + ...(anchor.side === "new" + ? { + suggestionAnchor: { + path: anchor.path, + startLine: anchor.startLine, + endLine: anchor.endLine, + expected: anchor.revisionText + .replace(/\n$/, "") + .split("\n") + .slice(anchor.startLine - 1, anchor.endLine) + .join("\n"), + }, + } + : {}), + }, + h, + ), + ); + return items.length ? h.div([], items) : undefined; }; - -const modeControl = (model: Model, h: HtmlBuilder): Html => h.div([ - h.Class("review-demo__mode"), - h.Role("group"), - h.AriaLabel("Diff layout"), -], [ - h.button([ - h.Type("button"), h.DataAttribute("active", model.mode === "Split" ? "true" : "false"), - h.OnClick(Message.ChangedMode({ mode: "Split" })), h.AriaLabel("Side-by-side diff"), - ], [Icon.view({ icon: SplitSquareHorizontal, size: 14 }, h), "Split"]), - h.button([ - h.Type("button"), h.DataAttribute("active", model.mode === "Unified" ? "true" : "false"), - h.OnClick(Message.ChangedMode({ mode: "Unified" })), h.AriaLabel("Unified diff"), - ], [Icon.view({ icon: Rows3, size: 14 }, h), "Unified"]), -]); - -const thread = (comment: ReviewComment, h: HtmlBuilder): Html => h.article([ - h.Class("review-demo__thread"), - h.DataAttribute("resolved", comment.resolved ? "true" : "false"), -], [ - h.div([h.Class("review-demo__thread-heading")], [ - h.span([h.Class("review-demo__avatar")], [comment.author === "You" ? "YO" : comment.author.split(" ").map((part) => part[0]).join("")]), - h.div([], [h.strong([], [comment.author]), h.span([], [selectionLabel(comment)])]), - h.span([h.Class("review-demo__thread-status")], [comment.resolved ? "Resolved" : "Open"]), - ]), - h.p([], [comment.body]), - h.button([ - h.Type("button"), - h.Class("review-demo__resolve"), - h.OnClick(Message.ToggledResolved({ id: comment.id })), - ], [comment.resolved ? "Reopen thread" : "Resolve thread"]), -]); - -const composer = (model: Model, h: HtmlBuilder): Html => { - if (model.selectionPath === "" || model.selectionStartLine < 1) { - return h.div([h.Class("review-demo__comment-empty")], [ - Icon.view({ icon: MessageSquare, size: 18 }, h), - h.strong([], ["Comment on a line"]), - h.p([], ["Click a line for one comment, or drag across line numbers to select a range."]), - ]); +const canvas = (model: Model, h: HtmlBuilder): Html => { + const file = currentFile(model), + before = parseValue(file.baseText), + after = parseValue(file.text); + let diff = fileDiff(file); + const revealed = model.threads.find( + (t) => t.id === model.reviewUi.activeThreadId && t.anchor.path === file.path, + ); + if (revealed && mappedThread(model, revealed).state !== "outdated") + diff = diffFile({ + path: file.path, + oldText: file.baseText, + newText: file.text, + context: Math.max(file.baseText.length, file.text.length), + }); + for (const id of model.expandedGaps) { + const gap = contextGaps(diff).find((g) => g.beforeHunkId === id); + if (gap) diff = expandContext(diff, { oldText: file.baseText, newText: file.text }, gap, 20); } - return h.form([ - h.Class("review-demo__composer"), - h.OnSubmit(Message.SubmittedComment()), - ], [ - h.div([h.Class("review-demo__composer-heading")], [ - h.strong([], [selectionLabel({ - path: model.selectionPath, - side: model.selectionSide, - startLine: model.selectionStartLine, - endLine: model.selectionEndLine, - })]), - h.span([], [model.selectionPath.split("/").pop() ?? model.selectionPath]), - ]), - h.textarea([ - h.AriaLabel("Review comment"), - h.Placeholder("Leave a clear, actionable comment…"), - h.Value(model.draft), - h.OnInput((value) => Message.ChangedDraft({ value })), - ]), - h.div([h.Class("review-demo__composer-actions")], [ - Button.view({ label: "Cancel", size: "xs", variant: "ghost", onClick: Message.CancelledComment() }, h), - Button.view({ label: "Add comment", size: "xs", onClick: Message.SubmittedComment(), isDisabled: model.draft.trim() === "" }, h), - ]), - ]); + return h.main( + [h.Class("cr-demo__canvas")], + [ + h.section( + [h.Class("cr-demo__editor")], + [ + h.div( + [h.Class("cr-demo__section-heading")], + [ + h.h2( + [], + [model.actor === "author" ? "Working configuration" : "Proposed configuration"], + ), + h.span( + [], + [ + model.actor === "author" && model.status !== "merged" + ? "Editable · Effect schema" + : "Read only", + ], + ), + ], + ), + CodeEditor.view( + { + model: { + ...model.editor, + options: { + ...model.editor.options, + theme: model.dark ? "dark" : "light", + readOnly: model.actor !== "author" || model.status === "merged", + }, + }, + label: "Configuration source", + meta: file.path, + showToolbar: false, + showInspector: false, + toParentMessage: (message) => Message.GotEditorMessage({ message }), + }, + h, + ), + ], + ), + h.section( + [h.Class("cr-demo__diff")], + [ + h.div( + [h.Class("cr-demo__section-heading")], + [ + h.h2([], ["Review changes"]), + h.div( + [h.Class("cr-demo__toolbar"), h.Role("group"), h.AriaLabel("Diff layout")], + [ + button( + model.semantic ? "Line diff" : "Semantic values", + Message.ToggledSemantic(), + h, + ), + button("Unified diff", Message.ChangedMode({ mode: "Unified" }), h), + button("Side-by-side diff", Message.ChangedMode({ mode: "Split" }), h), + ], + ), + ], + ), + model.semantic && before !== undefined && after !== undefined + ? ValueDiff.view({ changes: diffValues(before, after) }, h) + : model.semantic + ? h.p([], ["Fix syntax before comparing decoded values."]) + : diff.hunks.length + ? DiffViewer.view( + { + file: diff, + highlight: reviewHighlight, + mode: model.mode, + ...(model.selection + ? { selectedRange: normalizeSelection(model.selection) } + : {}), + annotations: diagnosticsToAnnotations( + file.path, + file.text, + diagnostics(file), + ), + threads: model.threads.flatMap((t) => { + const mapping = mappedThread(model, t); + return mapping.state === "outdated" + ? [] + : [ + { + ...mapping.range, + count: t.comments.length, + resolved: t.status === "resolved", + }, + ]; + }), + reviewed: model.viewedPaths.includes(file.path), + onReviewedChange: (viewed) => + Message.ChangedViewed({ path: file.path, viewed }), + ...(model.status === "open" + ? { + onSelectLine: (ref: DiffLineRef) => Message.SelectedLine(ref), + onStartSelection: (ref: DiffLineRef) => Message.StartedSelection(ref), + onExtendSelection: (ref: DiffLineRef, method: "Pointer" | "Keyboard") => + Message.ExtendedSelection({ ...ref, method }), + onEndSelection: () => Message.EndedSelection(), + onCancelSelection: () => Message.CancelledSelection(), + } + : {}), + lineWidgets: (ref) => lineWidgets(model, ref, h), + onExpandGap: (gap) => Message.ExpandedGap({ id: gap.beforeHunkId }), + }, + h, + ) + : h.p( + [h.Class("cr-demo__empty")], + [ + model.status === "merged" + ? "Saved configuration matches the merged proposal. See the activity below." + : "Edit this file to compare it with the saved configuration.", + ], + ), + ], + ), + ChangeSetPreview.view( + { + label: "Local impact preview", + basis: + "This fixture changes checkout availability and the configuration approval requirement.", + consequences: [ + { + kind: "changed", + label: "Checkout rollout", + detail: "The local policy limits the proposed rollout to 25%.", + }, + ], + actions: [], + }, + h, + ), + TransactionTimeline.view( + { + label: "Change request activity", + entries: model.activity, + emptyText: "Open a change request to start its activity history.", + renderBody: (entry) => + entry.kind === "review" + ? h.p([], ["Reviewer intent recorded by the local host."]) + : undefined, + }, + h, + ), + ], + ); }; - -const inspector = (model: Model, h: HtmlBuilder): Html => { - const activeComments = model.comments.filter((comment) => comment.path === model.activePath); - const openCount = model.comments.filter((comment) => !comment.resolved).length; - return h.aside([h.Class("review-demo__inspector"), h.AriaLabel("Review details")], [ - h.section([h.Class("review-demo__summary")], [ - h.div([h.Class("review-demo__summary-heading")], [ - Icon.view({ icon: GitPullRequest, size: 17 }, h), - h.div([], [h.strong([], ["Review summary"]), h.span([], ["PR #148 · ready for review"])]), - ]), - h.p([], ["Adds line-anchored review comments and scopes Git reads for larger change sets."]), - h.div([h.Class("review-demo__checks")], [ - h.span([], [Icon.view({ icon: CircleCheck, size: 14 }, h), "3 checks passed"]), - h.span([], [`${openCount} open ${openCount === 1 ? "thread" : "threads"}`]), - ]), - ]), - h.section([h.Class("review-demo__comment-section")], [ - h.div([h.Class("review-demo__section-heading")], [ - h.strong([], ["Comments"]), - h.span([], [String(activeComments.length)]), - ]), - composer(model, h), - ...activeComments.map((comment) => thread(comment, h)), - ]), - ]); -}; - -const diffReviewView = (model: Model, h: HtmlBuilder): Html => { - const activeFile = reviewFiles.find((file) => file.path === model.activePath) ?? reviewFiles[0]; - if (activeFile === undefined) return h.div([h.Class("review-demo")], ["No diff fixture loaded."]); - const selectedRange: DiffSelectionRange | undefined = model.selectionPath === "" ? undefined : { - path: model.selectionPath, - side: model.selectionSide, - startLine: model.selectionStartLine, - endLine: model.selectionEndLine, - }; - const markers: DiffThreadMarker[] = model.comments.map((comment) => ({ - path: comment.path, - side: comment.side, - startLine: comment.startLine, - endLine: comment.endLine, - count: 1, - resolved: comment.resolved, - })); - const activeIndex = reviewFiles.findIndex((file) => file.path === activeFile.path); - - return h.div([h.Class("review-demo"), h.DataAttribute("diff-review-demo", "true")], [ - h.header([h.Class("review-demo__topbar")], [ - h.div([h.Class("review-demo__crumbs")], [ - h.span([], ["foldworks"]), Icon.view({ icon: ChevronRight, size: 12 }, h), - h.span([], ["feature/inline-reviews"]), Icon.view({ icon: ChevronRight, size: 12 }, h), - h.strong([], ["Add inline code review comments"]), - ]), - h.div([h.Class("review-demo__topbar-actions")], [ - h.span([h.Class("review-demo__position")], [`${activeIndex + 1} / ${reviewFiles.length}`]), - modeControl(model, h), - ]), - ]), - h.div([h.Class("review-demo__workspace")], [ - navigation(model, h), - h.main([h.Class("review-demo__canvas")], [ - h.div([h.Class("review-demo__diff-scroll")], [ - DiffViewer.view({ - file: activeFile, - mode: model.mode, - ...(selectedRange === undefined ? {} : { selectedRange }), - threads: markers, - reviewed: model.viewedPaths.includes(activeFile.path), - onSelectLine: (selection) => Message.SelectedLine(selection), - onStartSelection: (selection) => Message.StartedSelection(selection), - onExtendSelection: (selection, method) => Message.ExtendedSelection({ ...selection, method }), - onEndSelection: () => Message.EndedSelection(), - onCancelSelection: () => Message.CancelledComment(), - onReviewedChange: (viewed) => Message.ToggledViewed({ path: activeFile.path, viewed }), - }, h), - ]), - ]), - inspector(model, h), - ]), - ]); -}; - -export const view = defineView((model, h) => diffReviewView(model, h)); +const inspector = (model: Model, h: HtmlBuilder): Html => + h.aside( + [h.Class("cr-demo__inspector"), h.AriaLabel("Review details")], + [ + CheckList.view( + { + checks: checks(model), + onRerun: () => Message.ReranChecks(), + renderDetails: (check) => + h.p( + [], + [ + check.id === "schema" + ? "Synchronous Effect schema validation of JSON source. Runs in this browser." + : "An illustrative local rule, evaluated in this browser. The host owns this policy.", + ], + ), + }, + h, + ), + ReviewerList.view( + { + reviewers: [ + { + author: reviewer, + required: true, + ownedFileCount: 2, + ...(model.reviews.at(-1) ? { verdict: model.reviews.at(-1)!.verdict } : {}), + }, + ], + }, + h, + ), + ...(model.status === "open" && model.actor === "reviewer" + ? [ + ReviewSubmit.view( + { + summary: model.summary, + verdict: model.verdict, + pendingCount: model.threads + .flatMap((t) => t.comments) + .filter((c) => c.pending && c.author.id === model.actor).length, + allowedVerdicts: ["comment", "approve", "request-changes"], + onSummaryChange: (value) => Message.ChangedSummary({ value }), + onVerdictChange: (verdict) => Message.ChangedVerdict({ verdict }), + onSubmit: Message.SubmittedReview(), + }, + h, + ), + ] + : []), + MergeReadiness.view( + { + requirements: [ + { + id: "checks", + label: "Local checks", + outcome: checks(model).every((c) => c.status === "passed") ? "met" : "unmet", + reason: "Both local validators must pass.", + children: checks(model).map((c) => ({ + id: c.id, + label: c.name, + outcome: c.status === "passed" ? "met" : "unmet", + reason: c.summary ?? "", + })), + }, + { + id: "approval", + label: "Current revision approved", + outcome: approved(model) ? "met" : "unmet", + reason: "The reviewer must approve this exact configuration revision.", + }, + { + id: "threads", + label: "Current discussions resolved", + outcome: openCurrentThreads(model).length ? "unmet" : "met", + reason: + "Outdated discussions remain in history and do not block this illustrative policy.", + }, + { + id: "pending", + label: "Pending comments published", + outcome: model.threads.some((t) => t.comments.some((c) => c.pending)) + ? "unmet" + : "met", + reason: "Submit pending review comments.", + }, + ], + canMerge: canMerge(model), + onMerge: Message.Merged(), + strategies: [{ id: "apply", label: "Apply configuration" }], + strategy: "apply", + onStrategyChange: () => Message.CompletedReveal(), + }, + h, + ), + ReviewThreads.listView( + { + model: model.reviewUi, + threads: model.threads, + groupBy: (t) => t.anchor.path, + anchorState: (t) => mappedThread(model, t).state, + toMessage: (message) => Message.GotThreadsMessage({ message }), + }, + h, + ), + ...model.threads + .filter( + (t) => + model.status === "merged" || + mappedThread(model, t).state === "outdated" || + !fileDiff(model.files.find((f) => f.path === t.anchor.path)!).hunks.length, + ) + .map((t) => threadView(model, t, h)), + ], + ); +export const view = defineView((model, h) => + h.div( + [ + h.Class("cr-demo"), + h.DataAttribute("change-request-demo", "true"), + h.DataAttribute("dark", String(model.dark)), + ], + [ + h.div( + [h.Class("cr-demo__notice")], + [ + h.strong([], ["LOCAL SIMULATION"]), + h.span( + [], + [ + "Synthetic configuration repository · stored in this browser · no Git provider integration", + ], + ), + ], + ), + ChangeRequestHeader.view( + { + title: "Roll out the new checkout", + status: model.status, + author, + baseLabel: "saved configuration", + headLabel: "working proposal", + actions: [ + button(model.dark ? "Light appearance" : "Dark appearance", Message.ToggledDark(), h), + button("Reset simulation", Message.Reset(), h), + ], + }, + h, + ), + h.div( + [h.Class("cr-demo__toolbar")], + [ + h.label( + [], + [ + "Configuration file ", + h.select( + [ + h.AriaLabel("Configuration file"), + h.Value(model.activePath), + h.OnChange((path) => Message.SelectedFile({ path })), + ], + model.files.map((file) => h.option([h.Value(file.path)], [file.path])), + ), + ], + ), + h.span( + [h.Class("cr-demo__actor")], + [`Acting as ${model.actor === "author" ? "author" : "reviewer"}`], + ), + button( + model.actor === "author" ? "Act as reviewer" : "Act as author", + Message.SwitchedActor({ actor: model.actor === "author" ? "reviewer" : "author" }), + h, + model.status === "draft", + ), + ...(model.status === "draft" + ? [ + button( + "Open change request", + Message.OpenedRequest(), + h, + !changedFiles(model).length || checks(model)[0]!.status !== "passed", + ), + ] + : []), + ], + ), + h.p( + [h.Class("cr-demo__hint")], + [ + model.status === "draft" + ? "Edit checkout to true and rollout to 100; try a string for checkout to see schema diagnostics. Then open a change request." + : "Review tip: select new lines 3–4 and suggest rollout 25 with a reviewedBy field. Arrow keys extend a range; Mod+Enter saves a pending comment.", + ], + ), + h.div( + [h.Class("cr-demo__workspace")], + [ + h.aside( + [h.Class("cr-demo__navigation"), h.AriaLabel("File navigation")], + [ + ChangedFiles.view( + { + model: model.filesUi, + files: changedFiles(model), + toMessage: (message) => Message.GotFilesMessage({ message }), + }, + h, + ), + h.p( + [], + [`${model.viewedPaths.length} files viewed · ${model.threads.length} discussions`], + ), + ], + ), + canvas(model, h), + inspector(model, h), + ], + ), + h.p( + [h.Role("status"), h.AriaLive("polite"), h.Class("cr-demo__announcement")], + [model.announcement], + ), + ...(model.storageError ? [h.p([h.Role("alert")], [model.storageError])] : []), + ], + ), +); diff --git a/apps/demo/src/docs/packages.ts b/apps/demo/src/docs/packages.ts index d0966d8..88cf1d7 100644 --- a/apps/demo/src/docs/packages.ts +++ b/apps/demo/src/docs/packages.ts @@ -34,6 +34,7 @@ const icons: Readonly> = { "data-table": Table2, diagram: Network, "diff-viewer": FileDiff, + review: FileDiff, editor: PenLine, "form-builder": TextCursorInput, "generative-ui": Sparkles, diff --git a/apps/demo/src/document-analysis/subscriptions.ts b/apps/demo/src/document-analysis/subscriptions.ts index 27c6e9a..3bf930d 100644 --- a/apps/demo/src/document-analysis/subscriptions.ts +++ b/apps/demo/src/document-analysis/subscriptions.ts @@ -1,8 +1,9 @@ +import { Option } from "effect"; import { Subscription } from "foldkit"; import * as Graph from "@foldworks/diagram/graph"; import type { Model } from "./model"; import { Message } from "./message"; export const subscriptions = Subscription.lift(Graph.subscriptions)({ - toChildModel: (model) => model.graph, + read: (model) => Option.some(model.graph), toParentMessage: (message) => Message.GotGraphMessage({ message }), }); diff --git a/apps/demo/src/home/view.ts b/apps/demo/src/home/view.ts index d71f2fd..e12da32 100644 --- a/apps/demo/src/home/view.ts +++ b/apps/demo/src/home/view.ts @@ -134,6 +134,11 @@ const packages: Readonly> = { href: workflowPath("Vertical"), icon: Workflow, }, + review: { + description: "Compose local change requests with anchored threads and host-owned persistence.", + href: diffViewerRouter(), + icon: FileDiff, + }, "diff-viewer": { description: "Review-grade unified and split diffs with line selection, threads, and progress.", href: diffViewerRouter(), diff --git a/apps/demo/src/stack.ts b/apps/demo/src/stack.ts index 4b8d4a2..a43906a 100644 --- a/apps/demo/src/stack.ts +++ b/apps/demo/src/stack.ts @@ -57,6 +57,7 @@ export const layers: ReadonlyArray = [ "form-builder", "workflow", "diff-viewer", + "review", "pdf-viewer", "pdf-annotator", ], diff --git a/apps/demo/src/styles.css b/apps/demo/src/styles.css index 7eec5a0..bfcccf8 100644 --- a/apps/demo/src/styles.css +++ b/apps/demo/src/styles.css @@ -13,6 +13,7 @@ @import "@foldworks/data-grid/styles.css"; @import "@foldworks/data-table/styles.css"; @import "@foldworks/diff-viewer/styles.css"; +@import "@foldworks/review/styles.css"; @import "@foldworks/pdf-annotator/styles.css"; @import "@foldworks/pdf-viewer/styles.css"; @import "./diff-viewer/styles.css"; diff --git a/docs/code-review.md b/docs/code-review.md new file mode 100644 index 0000000..8f828f0 --- /dev/null +++ b/docs/code-review.md @@ -0,0 +1,283 @@ +# Code review and change-request primitives + +A configuration-as-code system stores product configuration (forms, rules, +workflows, access policies, feature flags) as files and changes it through +reviewed proposals. Each team that builds one ends up rebuilding the same loop: + +**edit → see the change → open a change request → discuss it line by line → +suggest fixes → run checks → approve → merge → read the history.** + +This note describes the Foldworks primitives for that loop, so a host can compose +its own review process instead of building a GitHub clone. A host may read from +local Git, from GitHub or GitLab, from an agent's proposed edits, or from a +database of versioned documents. Foldworks never names a provider. The scopes +below are implemented and exercised by the local reference demo described at the +end. + +## Boundary + +The [language workbench principles](./language-workbench.md#principles) apply: +surfaces own geometry, focus, keys, and drafts; hosts own meaning and policy. + +| Layer | Owns | +| --------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Foldworks | Producing and rendering diffs, line and range selection, comment threads and their drafts, mapping anchors to new revisions, suggested-change math, and views for checks, reviewers, and merge requirements | +| Host | Repository transport, storage, identity, permissions, notifications, CODEOWNERS-style ownership, which checks run and how, merge policy evaluation, the merge itself, and domain-specific validation | + +The views render what the host provides and report user intent as messages. A +Foldworks view never decides whether a change may merge. It shows the +requirements the host evaluated and their outcomes. + +## Already available + +| Need | Foldworks API | +| ------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------ | +| Render a Git patch, unified or split | `parseUnifiedDiff` and `DiffViewer.view` in `@foldworks/diff-viewer`, with line and range selection, thread count markers, and a reviewed toggle | +| Compare prose | `diffText` and `ProseDiff.view` in `@foldworks/diff-viewer` | +| Compare structure | `diffTrees` and `TreeDiff.view` in `@foldworks/ui` | +| Summarize consequences and history | `ChangeSetPreview`, `TransactionTimeline`, and `ExplanationTree` in `@foldworks/ui` | +| Edit configuration with schema diagnostics | `CodeEditor` and `withSchema`/`validate` in `@foldworks/code-editor/structured` | +| Undo and redo | `@foldworks/history` | +| Browse a real checkout | `@foldworks/codebase` (read-only server and workbench) | +| File trees, split panes, command palette | `Tree`, `Workspace`, `SplitView`, and `Stateful.Command` in `@foldworks/ui` | +| Mentions in comment drafts | `Mentions` in `@foldworks/text-intelligence` | +| Agent-proposed edits | `@foldworks/agent` permission checkpoints with `allowLabel`/`denyLabel` and `renderDetails` | + +The [change-request demo](../apps/demo/src/diff-viewer/) composes the reusable +review submodel with host-owned configuration, viewed files, review verdicts, +revision snapshots and local persistence. + +## Implemented primitives + +### 1. Diffs from two texts, and richer line rendering + +Package: `@foldworks/diff-viewer`. + +A configuration host usually has a saved version and a draft, not a Git patch. + +- **`diffFile({ path, oldPath?, oldText, newText, context? })`** returns the same + `DiffFile` that `parseUnifiedDiff` produces: a line-level Myers diff with + hunks, headers, and `context` unchanged lines (default 3), status + `added`/`deleted`/`modified`/`renamed`, and the same bounded search fallback as + `diffText`. A pure `formatUnifiedDiff(file)` turns it back into patch text, so + hosts can store or send it. +- **Intraline highlights.** Paired deletion and addition lines in a replacement + block highlight their changed words using `diffText`. The view keeps semantic + ``/`` markup for the changed spans. +- **Line slots.** `lineWidgets?: (line: DiffLineRef) => Html | undefined` renders host Html below a line in unified and split modes, + spanning the full row. Inline threads, composers, and check annotations go + here. `DiffLineRef` is `{ path, side, line }`. +- **Line annotations.** `annotations?: readonly DiffAnnotation[]`, each + `{ path, side, startLine, endLine, tone, label, message? }`, marks lines with a + gutter tone and an accessible label. Schema diagnostics, lint results, and + check failures map to these. +- **Expandable context.** Hidden lines between hunks render as an expander when + the host supplies `onExpandGap(gap)`. A pure `expandContext(file, { oldText, +newText }, gap, lines)` returns a new `DiffFile` with those lines added as + context, so expansion stays host-owned data. +- **Highlighting hook.** `highlight?: (content, language) => readonly Highlight[]` + replaces the built-in regex tokenizer when provided. It uses the + `@foldworks/text-intelligence` semantic token kinds so the code editor's + language service can serve both. +- **Mapping anchors to a new revision.** `mapLine(file, side, line)` and + `mapRange(fromFile, range, toFile)` report where a line on one side lands on + the other, or that it was deleted. A thread created on revision 1 can then be + shown on revision 2 as current, moved, or outdated. `mapRange` always targets + the new side and returns `{ state, range }` or `{ state: "outdated", reason }`. + `diffFile` retains source snapshots for exact revision mapping; parsed patches + without snapshots require a common base and cannot remap added lines. + Replacements and noncontiguous ranges become outdated. Context expansion + supports gaps before/between hunks, not the trailing file region. + +### 2. Review threads + +Package: new `@foldworks/review`. This is the +[anchored review threads](./planned-primitives.md#anchored-review-threads) +exploration, with a diff line as the first anchor and the anchor type left +generic so document ranges, grid cells, diagram nodes, and PDF regions can use +it later. + +- **Schemas.** `Comment` `{ id, author: { id, name, avatarUrl? }, body, +createdAt, editedAt?, pending? }` and `Thread` `{ id, anchor, status: +"open" | "resolved", comments }`. The thread's display state + (`current | moved | outdated`) is computed by the host or by the diff-viewer + mapping helpers, not stored. +- **Submodel.** `ReviewThreads` owns UI state only: which composer is open, draft + text per thread and per new anchor, which comment is being edited, and which + resolved threads are expanded. It follows the existing `Model`/`Message`/ + `update` and `OutMessage` pattern. The host receives + `RequestedCreateThread`, `RequestedReply`, `RequestedEdit`, `RequestedDelete`, + `RequestedResolve`, and `RequestedReopen`, and answers by updating the thread + data it passes back in. Drafts survive a host round trip and are cleared only + when the host confirms with `Committed({ draftKey, body })`, so a failed save keeps + the text. The acknowledgement includes the saved body so a delayed response + cannot erase newer typing. New-anchor requests carry a host-owned `anchorKey`; + hosts keep the generic anchor data outside the UI submodel. Optional suggestion + composers keep replacement drafts in this submodel too; a matching save + acknowledgement also includes the saved suggestion. +- **Views.** `ReviewThreads.threadView` renders one thread with its comments, a + reply composer, and resolve/reopen. `ReviewThreads.composerView` renders the + new-thread composer for a selected range. `ReviewThreads.listView` renders all + threads grouped by file with open/resolved/outdated filters and an activation + message to reveal the anchor. Comment bodies render through a host + `renderBody` (default: preserved plain text), so Markdown rendering, mentions, + and links stay host choices. +- **Keyboard and focus.** The composer submits with Mod+Enter and cancels with + Escape, returning focus to the gutter that opened it. Thread lists are a + single tab stop with arrow-key movement. Status changes are announced through + a polite live region. + +### 3. Suggested changes + +Package: `@foldworks/review`, using `@foldworks/diff-viewer` for rendering. + +A suggestion is a comment that carries replacement text for its anchored range +on the new side. + +- `Suggestion` `{ path, startLine, endLine, replacement, expected }` is an optional field + on `Comment`. +- `applySuggestions(text, suggestions)` applies one or more suggestions to a + file's text, rejects overlapping ranges, and reports suggestions whose range + no longer matches (`stale`) rather than guessing. `expected` is the original + range content joined with LF, without its terminal newline. Batches are atomic + and cover one path. Application normalizes CRLF to LF and preserves the source + final-newline convention. Empty replacement deletes complete lines; a single + terminal newline on a nonempty replacement is ignored. +- The comment view shows the suggestion as a small diff and offers "Apply" when + the host passes `onApplySuggestion`. Applying is a host action: the host + commits the new text and resolves the thread. + +### 4. Review submission + +Package: `@foldworks/review`. + +- Comments may be `pending` until the reviewer submits a review. `ReviewSubmit` + is a view with a summary composer, the pending count, and a verdict choice of + `comment | approve | request-changes`. The host decides which verdicts the + current user may choose and passes the allowed set. +- `ReviewerList` shows requested reviewers and reviewers who responded, with a + verdict tone per reviewer and optional "required" and "owner of N files" + labels supplied by the host. + +### 5. Change-request chrome + +Package: `@foldworks/review`. + +- **`ChangedFiles`.** A file tree built on `Tree`: status icons, additions and + deletions, viewed checkboxes, open-thread counts, annotation tones, a text + filter, and "show only unviewed". Directory rows aggregate counts. Selecting a + file sends `SelectedFile`. A flat list mode suits small changes. +- **`CheckList`.** Rows of `{ id, name, status: queued | running | passed | +failed | skipped | cancelled | neutral, summary?, duration?, detailsHref? }` + with an aggregate summary ("2 failed, 5 passed"), a re-run action when the + host provides one, and expandable host-rendered details. Checks are whatever + the host runs: schema validation, policy evaluation, a dry-run preview, or CI. +- **`MergeReadiness`.** The requirements the host evaluated, each with outcome + `met | unmet | pending | bypassable`, a reason, and an optional action. A merge + button with an optional strategy menu (merge, squash, rebase, or host-defined + labels) is enabled only when the host says so. It reuses `ExplanationTree` + for nested requirements. +- **`ChangeRequestHeader`.** Title, status (`draft | open | merged | closed`), + author, base and head labels, and actions. It composes `AppHeader`-level + pieces rather than replacing them. +- **Activity.** `TransactionTimeline` covers applied changes. A change request + also needs comments, reviews, pushes, check results, and status changes in + one stream. `TransactionTimeline` accepts an open-string entry `kind` and a + `renderBody(entry)` host slot, retaining its existing change rows and context. + +### 6. Configuration-aware review + +Packages: `@foldworks/diff-viewer` for the pure parts, the demo for the +composition. + +Line diffs are honest but do not say what changed in a configuration value. + +- **`diffValues(before, after)`** compares two decoded JSON-like values and + returns path-level changes `{ path: readonly (string | number)[], kind: +"added" | "removed" | "changed", before?, after? }`. Arrays of objects with an + `id` (or a host `key` function) are matched by identity; other arrays are + compared by index. Identity-matched arrays ignore pure reorder; changed/added + paths use after indexes and removed paths use before indexes. This is a review + summary, not a patch. Inputs must be finite, acyclic JSON-like values. +- **`ValueDiff.view`** renders those changes grouped by path with before and + after values, as a "semantic" alternative to the line view of the same file. + A host that has source maps from its parser can link a path change to its + lines through `annotations`. +- **Diagnostics in review.** A helper converts `@foldworks/code-editor` + `Diagnostic`s (offset ranges in the new text) into new-side + `DiffAnnotation`s, so schema errors from `withSchema` appear in the review + without the host re-implementing offset-to-line math. +- **Impact.** A host that can compute consequences of the change (affected + users, generated tasks, a plan like `terraform plan`) shows them with + `ChangeSetPreview`. No new component. + +## Reference demo + +The implementation extends the existing [diff-viewer demo](../apps/demo/src/diff-viewer/) +with a **change request** reference app on a synthetic, browser-local +configuration repository: a few YAML or JSON files for feature flags and access +rules, with an Effect schema for each. It says on screen that it is a local +simulation with no Git provider integration. + +The scenario, also the e2e test (`change request reference`): + +1. Open a file and edit a flag in `CodeEditor`; a schema error shows inline. +2. Fix it; the working change shows as a diff produced by `diffFile`, with the + semantic `ValueDiff` view available as a toggle. +3. Open a change request; `ChangedFiles`, `CheckList` (local schema and policy + validators, labeled as such), and `MergeReadiness` reflect it. +4. As a second reviewer, select lines, leave a pending comment and a suggestion, + and submit "Request changes". +5. As the author, apply the suggestion; the thread moves to the new revision + and an unrelated thread becomes outdated. The host explicitly reanchors the + applied suggestion to its replacement range and retains its original revision. +6. Approve, see every requirement met, merge, and read the activity timeline. + State survives a reload. The illustrative policy requires current-revision + approval and resolved current discussions; outdated discussions remain + nonblocking history. Storage failures retain the in-memory session; multi-tab + conflict control is deferred for this local reference adapter. + +Also cover keyboard-only commenting, narrow screens, and dark mode, following +the [workers workbench](./workbench/README.md) conventions for screenshots. + +## Sequence + +All seven steps ship with tests, README documentation of behavior and limits, a +changeset, and demo coverage: + +1. `diffFile`, `formatUnifiedDiff`, intraline highlights, line slots, + annotations, and the highlighting hook in `@foldworks/diff-viewer`. +2. `@foldworks/review` with `Comment`, `Thread`, the `ReviewThreads` submodel, + thread, composer, and list views, wired into the demo through line slots. +3. `mapLine`/`mapRange` and outdated threads; `expandContext`. +4. Suggestions and `applySuggestions`; pending comments, `ReviewSubmit`, and + `ReviewerList`. +5. `ChangedFiles`, `CheckList`, `MergeReadiness`, `ChangeRequestHeader`, and the + timeline extension. +6. `diffValues`, `ValueDiff.view`, and the diagnostics-to-annotations helper. +7. The full change-request reference demo and its e2e scenario. + +## Reference verification + +`pnpm check` validates each implementation step. `pnpm test:e2e` includes the full +local review flow plus keyboard cancellation, narrow layout and dark appearance. +Captures are saved to `.context/demo-screenshots/change-request-*.png` with +animations disabled. These are documentation captures, not pixel comparisons. +See [demo instructions](../apps/demo/README.md#local-change-request-reference). + +## Later + +These wait for a host that needs them. + +- **Three-way merge and conflict resolution.** A view that shows base, ours, + and theirs and lets the user choose or edit each conflict. +- **Revision picker and commit list.** Choosing base and head, and comparing + any two revisions of a change request. +- **Blame.** Line attribution beside source in `@foldworks/codebase`. +- **Very large diffs.** Virtualized hunks and lazy file loading. The data grid + virtualization is the model. +- **Presence and realtime.** Who is viewing, live comment arrival, and typing + indicators. Transport stays with the host. +- **Thread anchors outside diffs.** Document ranges in `@foldworks/editor`, + grid cells, diagram nodes, and PDF regions, designed with the + [anchors that survive edits](./document-analysis.md#later) work. diff --git a/docs/planned-primitives.md b/docs/planned-primitives.md index e55be0e..f49bc22 100644 --- a/docs/planned-primitives.md +++ b/docs/planned-primitives.md @@ -51,6 +51,8 @@ ranges, diff lines, grid cells or rows, diagram nodes, and PDF regions. Include thread creation, replies, resolution, focus, and anchors that can become stale after edits. Keep transport, permissions, and persistence in the host app. The diff viewer already renders thread markers, but does not own thread interaction. +[Code review and change-request primitives](./code-review.md) proposes the +first implementation, with diff lines as the first anchor. ## Further explorations diff --git a/packages/diagram/src/graph.ts b/packages/diagram/src/graph.ts index c4aa586..2f6ef48 100644 --- a/packages/diagram/src/graph.ts +++ b/packages/diagram/src/graph.ts @@ -296,7 +296,7 @@ export const update = (model: Model, message: Message, document: GraphDocument): }); const canvasSubscriptions = Subscription.lift(Diagram.subscriptions)({ - toChildModel: (model) => model.canvas, + read: (model) => Option.some(model.canvas), toParentMessage: (message) => Message.GotCanvasMessage({ message }), }); diff --git a/packages/diff-viewer/README.md b/packages/diff-viewer/README.md index ee434c5..77471db 100644 --- a/packages/diff-viewer/README.md +++ b/packages/diff-viewer/README.md @@ -64,3 +64,82 @@ prefix and suffix tokens remain equal. This retains exact source text while avoiding unbounded diff-search memory on long, unrelated inputs. The view is controlled and read-only, uses semantic ``/`` markup and theme tones, and needs the same package stylesheet as `DiffViewer`. + +## Text sources and line extensions + +`diffFile({ path, oldPath?, oldText, newText, context: 3 })` produces a bounded +Myers line diff without requiring a patch. `formatUnifiedDiff(file)` serializes +it. Empty sources imply added/deleted status; `oldPath` selects rename status. +CRLF is normalized to LF. Missing final newlines are represented by +`DiffLine.noNewline` and preserved in patches. Paths must be plain paths without +newlines or quoted patch escapes. Binary content is not inferred. Search uses +`maxSearchCells` (default one million), then falls back to a replacement. + +`lineWidgets(ref)` inserts host Html below a line, spanning both columns in +split mode. Context rows expose both old and new refs; return a widget for only +the side on which it belongs. `annotations` adds gutter tones and accessible +labels, with optional messages beneath the range's last line. `lineGutterId` +provides a stable focus-return target. Paired replacement rows use semantic +`ins`/`del` word highlights; unequal blocks pair by index, without move detection. + +`highlight(content, language)` can supply text-intelligence `SemanticToken` +ranges (UTF-16, exclusive end, open string kinds). Invalid or overlapping tokens +are ignored. The hook receives complete lines even on replacement rows; token ranges are +clipped to unchanged fragments and changed spans retain their change markup. The default is a small regex tokenizer. + +## Revision anchors and context + +`mapLine(file, side, line)` returns `mapped` with the opposite-side line, +`deleted` for removed/replaced lines, or `unknown` for invalid coordinates. +Unchanged lines outside hunks follow cumulative offsets. Parsed patches cannot +validate coordinates beyond the represented source; hosts must supply valid +line numbers. + +`mapRange(fromFile, range, toFile)` maps to the later file's **new** side and +returns `{ state: 'current' | 'moved', range }` or +`{ state: 'outdated', reason }`. Every source line must survive contiguously. +`diffFile` retains full `source` snapshots, allowing exact revision-to-revision +mapping even for added lines. Serialization omits snapshots. Without both +snapshots, mapping requires a shared base path and surviving base-side lines; +newly added anchors conservatively become outdated. No fuzzy or move matching. +Hosts retain original anchors/snapshots for outdated threads and decide which +revision to reveal. + +`contextGaps(file)` describes hidden context before/between hunks. Supply +`onExpandGap` to render an expansion button. +`expandContext(file, { oldText, newText }, gap, lines)` adds context preceding +the next hunk and joins touching hunks. It validates visible source content and +requested gap lines, rejecting stale gaps or mismatched sources. It does not +expand the trailing file region. Counts and file totals remain unchanged. + +## Decoded configuration values and diagnostics + +`diffValues(before, after, { key? })` accepts finite, acyclic JSON-like values and +returns added/removed/changed paths. Objects compare own string keys. Arrays with +unique `id` values on every item match by identity; `key(value, arrayPath)` can +supply host identity. Duplicate/missing keys fall back to indexes. Pure reorder +in an identity-matched array is ignored. Matched/added paths use after indexes; +removed paths use before indexes. This is a review summary, not an executable +patch. Non-JSON values and cycles throw. Inputs should fit in memory and within +the JavaScript call stack; there is no streaming or depth virtualization. + +`ValueDiff.view({ changes, onSelectPath? }, h)` groups by parent path and renders +before/after JSON with semantic deletion/insertion markup. The optional path +callback lets a host use parser source maps; the package does not infer source +locations from decoded values. + +`diagnosticsToAnnotations(path, text, diagnostics)` accepts code-editor / +text-intelligence diagnostics with UTF-16 offsets and exclusive `to`. It clamps +offsets, handles CRLF and point diagnostics, and maps error/warning/info/hint to +gutter tones on the new side. EOF points map to the last real line (line 1 for +empty text); an empty diff has no line on which to paint the annotation, so show +file-level validation in the host checks view as well. + +## Change-request reference + +The demo at `/diff-viewer` compares synthetic JSON configuration with `diffFile`, +offers line/semantic views, and maps schema diagnostics into gutters. Review +threads and suggestions are composed through line slots using +`@foldworks/review`. The host retains source revisions and reanchors an applied +suggestion to its explicit replacement range; unrelated deleted anchors remain +outdated. This is a browser-local simulation. diff --git a/packages/diff-viewer/package.json b/packages/diff-viewer/package.json index 5fe9c41..35d15d1 100644 --- a/packages/diff-viewer/package.json +++ b/packages/diff-viewer/package.json @@ -56,5 +56,8 @@ "peerDependencies": { "effect": "catalog:peers", "foldkit": "catalog:peers" + }, + "dependencies": { + "@foldworks/text-intelligence": "workspace:*" } } diff --git a/packages/diff-viewer/src/anchors.test.ts b/packages/diff-viewer/src/anchors.test.ts new file mode 100644 index 0000000..f9ec7b0 --- /dev/null +++ b/packages/diff-viewer/src/anchors.test.ts @@ -0,0 +1,68 @@ +import { expect, it } from "vitest"; +import { diffFile, formatUnifiedDiff } from "./file"; +import { parseUnifiedDiff } from "./core"; +import { contextGaps, expandContext, mapLine, mapRange } from "./anchors"; + +it("maps both directions across insertions, deletions, hidden context and empty hunks", () => { + const file = diffFile({ + path: "f", + oldText: "a\nb\nc\nd\n", + newText: "start\na\nc\nd\nend\n", + context: 0, + }); + expect(mapLine(file, "old", 1)).toEqual({ status: "mapped", line: 2 }); + expect(mapLine(file, "old", 2)).toEqual({ status: "deleted" }); + expect(mapLine(file, "old", 3)).toEqual({ status: "mapped", line: 3 }); + expect(mapLine(file, "new", 1)).toEqual({ status: "deleted" }); + expect(mapLine(file, "new", 5)).toEqual({ status: "deleted" }); + expect(mapLine(file, "old", 5)).toEqual({ status: "unknown" }); + for (const oldText of ["a\nb\nc\n", "a\n", "", "a\nb\nc"]) + for (const newText of ["x\na\nb\nc\n", "b\nc\n", "a\nb\ny\nc\n", ""]) { + const diff = diffFile({ path: "f", oldText, newText, context: 0 }); + for (let line = 1; line <= oldText.split("\n").length; line++) { + const mapped = mapLine(diff, "old", line); + if (mapped.status === "mapped") + expect(mapLine(diff, "new", mapped.line)).toEqual({ status: "mapped", line }); + } + } +}); +it("maps new-side additions across revisions and rejects changed or noncontiguous ranges", () => { + const before = diffFile({ path: "f", oldText: "base\n", newText: "base\nadded\nkeep\n" }); + const after = diffFile({ + path: "f", + oldText: "base\n", + newText: "prefix\nbase\nadded\nchanged\n", + }); + expect(mapRange(before, { path: "f", side: "new", startLine: 2, endLine: 2 }, after)).toEqual({ + state: "moved", + range: { path: "f", side: "new", startLine: 3, endLine: 3 }, + }); + expect(mapRange(before, { path: "f", side: "new", startLine: 3, endLine: 3 }, after).state).toBe( + "outdated", + ); + const inserted = diffFile({ + path: "f", + oldText: "base\n", + newText: "base\nadded\ninsert\nkeep\n", + }); + expect( + mapRange(before, { path: "f", side: "new", startLine: 2, endLine: 3 }, inserted).state, + ).toBe("outdated"); + const parsed = parseUnifiedDiff(formatUnifiedDiff(before))[0]!; + expect(mapRange(parsed, { path: "f", side: "new", startLine: 2, endLine: 2 }, after).state).toBe( + "outdated", + ); +}); +it("expands supplied unchanged context, joins hunks and rejects stale source/gap data", () => { + const source = { oldText: "a\nb\nc\nd\ne\nf\ng\n", newText: "A\nb\nc\nd\ne\nf\nG\n" }; + const file = diffFile({ path: "f", ...source, context: 0 }); + const gap = contextGaps(file)[0]!; + expect(gap.count).toBe(5); + const partial = expandContext(file, source, gap, 2); + expect(partial.hunks[1]!.lines[0]!.oldLine).toBe(5); + expect(() => expandContext(partial, source, gap, 2)).toThrow("Stale"); + const expanded = expandContext(partial, source, contextGaps(partial)[0]!, 99); + expect(expanded.hunks).toHaveLength(1); + expect(expanded.additions).toBe(file.additions); + expect(() => expandContext(file, { ...source, newText: "mismatch" }, gap, 2)).toThrow(); +}); diff --git a/packages/diff-viewer/src/anchors.ts b/packages/diff-viewer/src/anchors.ts new file mode 100644 index 0000000..4340c84 --- /dev/null +++ b/packages/diff-viewer/src/anchors.ts @@ -0,0 +1,181 @@ +import { normalizeSelection, type DiffFile, type DiffSelectionRange, type DiffSide } from "./core"; +import { diffFile, makeHunk, textLines } from "./file"; + +export type LineMapping = + | Readonly<{ status: "mapped"; line: number }> + | Readonly<{ status: "deleted" | "unknown" }>; +/** Exact monotone mapping; replaced lines are deleted, never guessed. */ +export const mapLine = (file: DiffFile, side: DiffSide, line: number): LineMapping => { + if (!Number.isInteger(line) || line < 1 || file.status === "binary") return { status: "unknown" }; + if ( + file.source && + line > textLines(side === "old" ? file.source.oldText : file.source.newText).length + ) + return { status: "unknown" }; + let delta = 0; + for (const hunk of file.hunks) { + const start = side === "old" ? hunk.oldStart : hunk.newStart; + const count = side === "old" ? hunk.oldCount : hunk.newCount; + if (line < start || (count === 0 && line <= start)) + return { status: "mapped", line: line + delta }; + for (const row of hunk.lines) { + if ((side === "old" ? row.oldLine : row.newLine) !== line) continue; + const target = side === "old" ? row.newLine : row.oldLine; + return target === null ? { status: "deleted" } : { status: "mapped", line: target }; + } + delta = + side === "old" + ? hunk.newStart + hunk.newCount - (hunk.oldStart + hunk.oldCount) + : hunk.oldStart + hunk.oldCount - (hunk.newStart + hunk.newCount); + // Zero-count hunk coordinates address the preceding line. + if (hunk.oldCount === 0) delta += side === "old" ? -1 : 1; + if (hunk.newCount === 0) delta += side === "old" ? 1 : -1; + } + return { status: "mapped", line: line + delta }; +}; +export type RangeMapping = + | Readonly<{ state: "current" | "moved"; range: DiffSelectionRange }> + | Readonly<{ state: "outdated"; reason: "deleted" | "unknown" }>; +/** Maps a revision anchor onto the new side of a later revision. */ +export const mapRange = ( + fromFile: DiffFile, + range: DiffSelectionRange, + toFile: DiffFile, +): RangeMapping => { + const normalized = normalizeSelection(range); + if ( + range.path !== fromFile.path || + (fromFile.path !== toFile.path && + fromFile.path !== toFile.oldPath && + fromFile.oldPath !== toFile.oldPath) || + !Number.isInteger(normalized.startLine) || + !Number.isInteger(normalized.endLine) + ) + return { state: "outdated", reason: "unknown" }; + const revisionDiff = + fromFile.source && toFile.source + ? diffFile({ + path: toFile.path, + oldText: range.side === "old" ? fromFile.source.oldText : fromFile.source.newText, + newText: toFile.source.newText, + context: 0, + }) + : undefined; + let start: number | undefined, previous: number | undefined; + for (let line = normalized.startLine; line <= normalized.endLine; line++) { + const base = + range.side === "old" ? { status: "mapped" as const, line } : mapLine(fromFile, "new", line); + const mapped = revisionDiff + ? mapLine(revisionDiff, "old", line) + : base.status === "mapped" && fromFile.oldPath === toFile.oldPath + ? mapLine(toFile, "old", base.line) + : base.status === "deleted" + ? base + : { status: "unknown" as const }; + if (mapped.status !== "mapped") return { state: "outdated", reason: mapped.status }; + if (previous !== undefined && mapped.line !== previous + 1) + return { state: "outdated", reason: "deleted" }; + start ??= mapped.line; + previous = mapped.line; + } + if (start === undefined || previous === undefined) + return { state: "outdated", reason: "unknown" }; + const mapped = { path: toFile.path, side: "new" as const, startLine: start, endLine: previous }; + return { + state: + range.path === mapped.path && + range.side === "new" && + normalized.startLine === start && + normalized.endLine === previous + ? "current" + : "moved", + range: mapped, + }; +}; + +export type DiffGap = Readonly<{ + path: string; + beforeHunkId: string; + oldStart: number; + newStart: number; + count: number; +}>; +export const contextGaps = (file: DiffFile): readonly DiffGap[] => + file.hunks.flatMap((hunk, index) => { + const previous = file.hunks[index - 1]; + const oldStart = previous ? previous.oldStart + previous.oldCount : 1; + const newStart = previous ? previous.newStart + previous.newCount : 1; + const count = Math.min(hunk.oldStart - oldStart, hunk.newStart - newStart); + return count > 0 ? [{ path: file.path, beforeHunkId: hunk.id, oldStart, newStart, count }] : []; + }); +/** Expands from the end of a gap toward the preceding hunk; rejects stale inputs. */ +export const expandContext = ( + file: DiffFile, + source: Readonly<{ oldText: string; newText: string }>, + gap: DiffGap, + lines: number, +): DiffFile => { + const actual = contextGaps(file).find((g) => g.beforeHunkId === gap.beforeHunkId); + if ( + !actual || + Object.keys(actual).some((key) => actual[key as keyof DiffGap] !== gap[key as keyof DiffGap]) + ) + throw new RangeError("Stale context gap"); + if (!Number.isInteger(lines) || lines < 1) + throw new RangeError("Expansion must be a positive line count"); + const old = textLines(source.oldText), + next = textLines(source.newText); + for (const hunk of file.hunks) + for (const row of hunk.lines) { + const check = (text: string | undefined) => + text !== undefined && + text.replace(/\n$/, "") === row.content && + !text.endsWith("\n") === !!row.noNewline; + if ( + (row.oldLine !== null && !check(old[row.oldLine - 1])) || + (row.newLine !== null && !check(next[row.newLine - 1])) + ) + throw new RangeError("Source does not match diff"); + } + const take = Math.min(lines, gap.count); + const oldStart = gap.oldStart + gap.count - take, + newStart = gap.newStart + gap.count - take; + const context = Array.from({ length: take }, (_, index) => { + const text = old[oldStart + index - 1]; + if (text === undefined || text !== next[newStart + index - 1]) + throw new RangeError("Gap is not unchanged context"); + return { + kind: "context" as const, + content: text.replace(/\n$/, ""), + oldLine: oldStart + index, + newLine: newStart + index, + ...(!text.endsWith("\n") ? { noNewline: true } : {}), + }; + }); + const index = file.hunks.findIndex((h) => h.id === gap.beforeHunkId); + const hunk = file.hunks[index]!; + const expanded = makeHunk(file.path, [...context, ...hunk.lines], oldStart, newStart); + const previous = file.hunks[index - 1]; + const joined = + take === gap.count && previous + ? [ + makeHunk( + file.path, + [...previous.lines, ...expanded.lines], + previous.oldStart, + previous.newStart, + ), + ] + : [expanded]; + return { + ...file, + hunks: [ + ...file.hunks.slice( + 0, + index - (joined.length === 1 && take === gap.count && previous ? 1 : 0), + ), + ...joined, + ...file.hunks.slice(index + 1), + ], + }; +}; diff --git a/packages/diff-viewer/src/core.ts b/packages/diff-viewer/src/core.ts index aa5cb17..eac1a07 100644 --- a/packages/diff-viewer/src/core.ts +++ b/packages/diff-viewer/src/core.ts @@ -6,6 +6,7 @@ export type DiffSide = "old" | "new"; export interface DiffLine { readonly kind: DiffLineKind; readonly content: string; + readonly noNewline?: boolean; readonly oldLine: number | null; readonly newLine: number | null; } @@ -29,6 +30,8 @@ export interface DiffFile { readonly additions: number; readonly deletions: number; readonly hunks: readonly DiffHunk[]; + /** Full sources retained by diffFile for exact cross-revision mapping. */ + readonly source?: Readonly<{ oldText: string; newText: string }>; } export interface DiffSelection { @@ -79,13 +82,17 @@ export const selectionLabel = (selection: DiffSelectionRange): string => { : `${prefix} lines ${normalized.startLine}–${normalized.endLine}`; }; -export const selectableLines = ( - file: DiffFile, - side: DiffSide, -): readonly number[] => [...new Set(file.hunks.flatMap((hunk) => hunk.lines.flatMap((line) => { - const number = side === "old" ? line.oldLine : line.newLine; - return number === null ? [] : [number]; -})))].sort((left, right) => left - right); +export const selectableLines = (file: DiffFile, side: DiffSide): readonly number[] => + [ + ...new Set( + file.hunks.flatMap((hunk) => + hunk.lines.flatMap((line) => { + const number = side === "old" ? line.oldLine : line.newLine; + return number === null ? [] : [number]; + }), + ), + ), + ].sort((left, right) => left - right); export const adjacentSelectableLine = ( file: DiffFile, @@ -106,12 +113,28 @@ const cleanPath = (path: string): string => { export const languageFromPath = (path: string): string => { const extension = path.split(".").pop()?.toLowerCase() ?? ""; - return ({ - ts: "typescript", tsx: "tsx", js: "javascript", jsx: "jsx", - json: "json", css: "css", scss: "scss", html: "html", md: "markdown", - yml: "yaml", yaml: "yaml", py: "python", rs: "rust", go: "go", - sh: "shell", toml: "toml", - } as Record)[extension] ?? "text"; + return ( + ( + { + ts: "typescript", + tsx: "tsx", + js: "javascript", + jsx: "jsx", + json: "json", + css: "css", + scss: "scss", + html: "html", + md: "markdown", + yml: "yaml", + yaml: "yaml", + py: "python", + rs: "rust", + go: "go", + sh: "shell", + toml: "toml", + } as Record + )[extension] ?? "text" + ); }; type MutableFile = { @@ -139,8 +162,14 @@ export const parseUnifiedDiff = (patch: string): readonly DiffFile[] => { const finishFile = () => { finishHunk(); if (file === undefined) return; - const additions = file.hunks.reduce((sum, hunk) => sum + hunk.lines.filter((line) => line.kind === "addition").length, 0); - const deletions = file.hunks.reduce((sum, hunk) => sum + hunk.lines.filter((line) => line.kind === "deletion").length, 0); + const additions = file.hunks.reduce( + (sum, hunk) => sum + hunk.lines.filter((line) => line.kind === "addition").length, + 0, + ); + const deletions = file.hunks.reduce( + (sum, hunk) => sum + hunk.lines.filter((line) => line.kind === "deletion").length, + 0, + ); const path = file.path || file.oldPath; files.push({ ...file, path, language: languageFromPath(path), additions, deletions }); file = undefined; @@ -172,8 +201,10 @@ export const parseUnifiedDiff = (patch: string): readonly DiffFile[] => { file.path = cleanPath(rawLine.slice("rename to ".length)); file.status = "renamed"; } else if (rawLine.startsWith("Binary files ")) file.status = "binary"; - else if (rawLine.startsWith("--- ")) file.oldPath = cleanPath(rawLine.slice(4)); - else if (rawLine.startsWith("+++ ")) file.path = cleanPath(rawLine.slice(4)); + else if (lines === undefined && rawLine.startsWith("--- ")) + file.oldPath = cleanPath(rawLine.slice(4)); + else if (lines === undefined && rawLine.startsWith("+++ ")) + file.path = cleanPath(rawLine.slice(4)); const hunk = /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@\s?(.*)$/.exec(rawLine); if (hunk !== null) { @@ -193,7 +224,12 @@ export const parseUnifiedDiff = (patch: string): readonly DiffFile[] => { lines = []; continue; } - if (lines === undefined || rawLine === "" || rawLine === "\\ No newline at end of file") continue; + if (rawLine === "\\ No newline at end of file") { + const previous = lines?.at(-1); + if (previous && lines) lines[lines.length - 1] = { ...previous, noNewline: true }; + continue; + } + if (lines === undefined || rawLine === "") continue; if (rawLine.startsWith("+")) { lines.push({ kind: "addition", content: rawLine.slice(1), oldLine: null, newLine }); newLine += 1; @@ -232,13 +268,18 @@ export const splitRows = (hunk: DiffHunk): readonly SplitRow[] => { for (let row = 0; row < Math.max(deletions.length, additions.length); row += 1) { const left = deletions[row]; const right = additions[row]; - rows.push({ ...(left === undefined ? {} : { left }), ...(right === undefined ? {} : { right }) }); + rows.push({ + ...(left === undefined ? {} : { left }), + ...(right === undefined ? {} : { right }), + }); } } return rows; }; -export const diffTotals = (files: readonly DiffFile[]): Readonly<{ additions: number; deletions: number }> => ({ +export const diffTotals = ( + files: readonly DiffFile[], +): Readonly<{ additions: number; deletions: number }> => ({ additions: files.reduce((sum, file) => sum + file.additions, 0), deletions: files.reduce((sum, file) => sum + file.deletions, 0), }); diff --git a/packages/diff-viewer/src/file.test.ts b/packages/diff-viewer/src/file.test.ts new file mode 100644 index 0000000..ac26657 --- /dev/null +++ b/packages/diff-viewer/src/file.test.ts @@ -0,0 +1,97 @@ +import { expect, it } from "vitest"; +import { inertHtml, type HtmlBuilder } from "foldkit/html"; +import { Scene } from "foldkit/test"; +import { diffFile, formatUnifiedDiff } from "./file"; +import { parseUnifiedDiff } from "./core"; +import { view } from "./view"; + +it("round trips line counts, blank lines, renames and missing final newlines", () => { + for (const oldText of ["", "a\n", "a\n\nb", "last"]) + for (const newText of ["", "a\nb\n", "a\n\nc", "last\n"]) { + const file = diffFile({ path: "config.json", oldText, newText }); + const parsed = parseUnifiedDiff(formatUnifiedDiff(file))[0]!; + expect(parsed).toMatchObject({ + path: file.path, + status: file.status, + additions: file.additions, + deletions: file.deletions, + }); + expect(parsed.hunks.map((h) => h.lines)).toEqual(file.hunks.map((h) => h.lines)); + } + expect(diffFile({ path: "new", oldPath: "old", oldText: "a", newText: "a" }).status).toBe( + "renamed", + ); +}); +it("merges nearby changes and bounds replacement searches", () => { + const file = diffFile({ + path: "x", + oldText: "a\nb\nc\nd\ne\n", + newText: "a\nB\nc\nD\ne\n", + context: 0, + }); + expect(file.hunks).toHaveLength(2); + expect( + diffFile({ path: "x", oldText: "a\nb\n", newText: "c\nd\n", maxSearchCells: 0 }).additions, + ).toBe(2); +}); +it("renders full row slots, accessible annotations, custom tokens and semantic intraline edits in both modes", () => { + const file = diffFile({ + path: "x.ts", + oldText: "const a = false;\n", + newText: "const a = true;\n", + }); + const h = inertHtml as unknown as HtmlBuilder; + for (const mode of ["Unified", "Split"] as const) { + const html = view( + { + file, + mode, + onSelectLine: () => "select", + lineWidgets: (ref) => (ref.side === "new" ? h.p([], ["Thread"]) : undefined), + annotations: [ + { + path: "x.ts", + side: "new", + startLine: 1, + endLine: 1, + tone: "danger", + label: "Invalid flag", + }, + ], + highlight: () => [{ from: 0, to: 5, kind: "keyword" }], + }, + h, + )!; + expect(Scene.findAll(html, "ins")).toHaveLength(1); + expect(Scene.findAll(html, "del")).toHaveLength(1); + expect(Scene.findAll(html, ".fk-diff-viewer__widget")).toHaveLength(1); + expect(Scene.findAll(html, '[data-annotation-tone="danger"]')).toHaveLength(1); + } +}); + +it("preserves header-looking source lines and binary status when serializing", () => { + const file = diffFile({ path: "config.txt", oldText: "-- old\n", newText: "++ new\n" }); + const parsed = parseUnifiedDiff(formatUnifiedDiff(file))[0]!; + expect(parsed.path).toBe("config.txt"); + expect(parsed.oldPath).toBe("config.txt"); + expect(parsed.hunks[0]!.lines.map((l) => l.content)).toEqual(["-- old", "++ new"]); + expect( + parseUnifiedDiff(formatUnifiedDiff({ ...file, status: "binary", hunks: [] }))[0]!.status, + ).toBe("binary"); +}); + +it("calls the highlighting hook with complete source lines for replacements", () => { + const seen: string[] = []; + const h = inertHtml as unknown as HtmlBuilder; + view( + { + file: diffFile({ path: "f", oldText: "prefix old suffix\n", newText: "prefix new suffix\n" }), + highlight: (content) => { + seen.push(content); + return [{ from: 0, to: content.length, kind: "custom" }]; + }, + }, + h, + ); + expect(seen).toEqual(["prefix old suffix", "prefix new suffix"]); +}); diff --git a/packages/diff-viewer/src/file.ts b/packages/diff-viewer/src/file.ts new file mode 100644 index 0000000..d32cc4a --- /dev/null +++ b/packages/diff-viewer/src/file.ts @@ -0,0 +1,136 @@ +import { languageFromPath, type DiffFile, type DiffHunk, type DiffLine } from "./core"; +import { myers } from "./myers"; + +export type DiffFileOptions = Readonly<{ + path: string; + oldPath?: string; + oldText: string; + newText: string; + context?: number; + maxSearchCells?: number; +}>; + +/** Split LF/CRLF text without inventing a line after the final newline. */ +export const textLines = (text: string): string[] => + text.replace(/\r\n/g, "\n").match(/[^\n]*\n|[^\n]+$/g) ?? []; + +export const makeHunk = ( + path: string, + lines: readonly DiffLine[], + oldStart: number, + newStart: number, +): DiffHunk => { + const oldCount = lines.filter((line) => line.kind !== "addition").length; + const newCount = lines.filter((line) => line.kind !== "deletion").length; + return { + id: `${path}:${oldStart}:${newStart}`, + header: `@@ -${oldStart},${oldCount} +${newStart},${newCount} @@`, + label: "", + oldStart, + oldCount, + newStart, + newCount, + lines, + }; +}; + +/** Bounded line-level Myers diff. Empty sources imply added/deleted status. */ +export const diffFile = (options: DiffFileOptions): DiffFile => { + const old = textLines(options.oldText); + const next = textLines(options.newText); + let prefix = 0; + while (prefix < Math.min(old.length, next.length) && old[prefix] === next[prefix]) prefix++; + let suffix = 0; + while ( + suffix < Math.min(old.length, next.length) - prefix && + old.at(-1 - suffix) === next.at(-1 - suffix) + ) + suffix++; + const budget = options.maxSearchCells ?? 1_000_000; + const edits = [ + ...old.slice(0, prefix).map((text) => ({ kind: "equal" as const, text })), + ...myers( + old.slice(prefix, old.length - suffix), + next.slice(prefix, next.length - suffix), + Number.isFinite(budget) ? Math.max(0, budget) : 1_000_000, + ), + ...old.slice(old.length - suffix).map((text) => ({ kind: "equal" as const, text })), + ]; + let oldLine = 1, + newLine = 1; + const positions: { old: number; next: number }[] = []; + const lines: DiffLine[] = edits.map((edit) => { + positions.push({ old: oldLine, next: newLine }); + return { + kind: edit.kind === "equal" ? "context" : edit.kind === "removal" ? "deletion" : "addition", + content: edit.text.replace(/\n$/, ""), + oldLine: edit.kind === "addition" ? null : oldLine++, + newLine: edit.kind === "removal" ? null : newLine++, + ...(!edit.text.endsWith("\n") ? { noNewline: true } : {}), + }; + }); + const context = Number.isFinite(options.context ?? 3) + ? Math.max(0, Math.floor(options.context ?? 3)) + : 3; + const intervals: { start: number; end: number }[] = []; + lines.forEach((line, index) => { + if (line.kind === "context") return; + const start = Math.max(0, index - context), + end = Math.min(lines.length, index + context + 1); + const previous = intervals.at(-1); + if (previous && previous.end >= start) previous.end = end; + else intervals.push({ start, end }); + }); + const hunks = intervals.map(({ start, end }) => { + const block = lines.slice(start, end); + const pos = positions[start]!; + return makeHunk( + options.path, + block, + pos.old - (block.every((l) => l.oldLine === null) ? 1 : 0), + pos.next - (block.every((l) => l.newLine === null) ? 1 : 0), + ); + }); + return { + source: { oldText: options.oldText, newText: options.newText }, + path: options.path, + oldPath: + options.oldPath ?? (options.oldText === "" && options.newText !== "" ? "" : options.path), + status: + options.oldPath !== undefined && options.oldPath !== options.path + ? "renamed" + : options.oldText === "" && options.newText !== "" + ? "added" + : options.newText === "" && options.oldText !== "" + ? "deleted" + : "modified", + language: languageFromPath(options.path), + additions: lines.filter((l) => l.kind === "addition").length, + deletions: lines.filter((l) => l.kind === "deletion").length, + hunks, + }; +}; + +export const formatUnifiedDiff = (file: DiffFile): string => + [ + `diff --git a/${file.oldPath || file.path} b/${file.path}`, + ...(file.status === "added" + ? ["new file mode 100644"] + : file.status === "deleted" + ? ["deleted file mode 100644"] + : file.status === "renamed" + ? [`rename from ${file.oldPath}`, `rename to ${file.path}`] + : []), + `--- ${file.status === "added" ? "/dev/null" : `a/${file.oldPath}`}`, + `+++ ${file.status === "deleted" ? "/dev/null" : `b/${file.path}`}`, + ...(file.status === "binary" + ? [`Binary files a/${file.oldPath || file.path} and b/${file.path} differ`] + : []), + ...file.hunks.flatMap((hunk) => [ + hunk.header, + ...hunk.lines.flatMap((line) => [ + `${line.kind === "addition" ? "+" : line.kind === "deletion" ? "-" : " "}${line.content}`, + ...(line.noNewline ? ["\\ No newline at end of file"] : []), + ]), + ]), + ].join("\n") + "\n"; diff --git a/packages/diff-viewer/src/index.ts b/packages/diff-viewer/src/index.ts index 02eedb2..ea90e83 100644 --- a/packages/diff-viewer/src/index.ts +++ b/packages/diff-viewer/src/index.ts @@ -1,5 +1,12 @@ export * from "./core"; -export { view, type ViewConfig } from "./view"; +export { + view, + lineGutterId, + type ViewConfig, + type Highlight, + type DiffAnnotation, + type DiffLineRef, +} from "./view"; export * as DiffViewer from "./view"; export { diffText, @@ -8,3 +15,7 @@ export { type ProseDiffConfig, type TextDiffPart, } from "./prose"; + +export * from "./file"; +export * from "./anchors"; +export * from "./values"; diff --git a/packages/diff-viewer/src/myers.ts b/packages/diff-viewer/src/myers.ts new file mode 100644 index 0000000..23aa6f6 --- /dev/null +++ b/packages/diff-viewer/src/myers.ts @@ -0,0 +1,56 @@ +type Edit = Readonly<{ kind: "equal" | "addition" | "removal"; text: string }>; +export const myers = ( + old: readonly string[], + next: readonly string[], + maxCells: number, +): Edit[] => { + const trace: Map[] = []; + let frontier = new Map([[1, 0]]); + let cells = 0; + for (let distance = 0; distance <= old.length + next.length; distance++) { + cells += 2 * distance + 1; + if (cells > maxCells) break; + trace.push(new Map(frontier)); + for (let diagonal = -distance; diagonal <= distance; diagonal += 2) { + let x = + diagonal === -distance || + (diagonal !== distance && + (frontier.get(diagonal - 1) ?? -Infinity) < (frontier.get(diagonal + 1) ?? -Infinity)) + ? (frontier.get(diagonal + 1) ?? 0) + : (frontier.get(diagonal - 1) ?? 0) + 1; + let y = x - diagonal; + while (x < old.length && y < next.length && old[x] === next[y]) { + x++; + y++; + } + frontier.set(diagonal, x); + if (x < old.length || y < next.length) continue; + const edits: Edit[] = []; + x = old.length; + y = next.length; + for (let depth = distance; depth >= 0; depth--) { + const previous = trace[depth]!; + const k = x - y; + const previousK = + k === -depth || + (k !== depth && (previous.get(k - 1) ?? -Infinity) < (previous.get(k + 1) ?? -Infinity)) + ? k + 1 + : k - 1; + const previousX = previous.get(previousK) ?? 0; + const previousY = previousX - previousK; + while (x > previousX && y > previousY) { + edits.push({ kind: "equal", text: old[--x]! }); + y--; + } + if (depth === 0) break; + if (x === previousX) edits.push({ kind: "addition", text: next[--y]! }); + else edits.push({ kind: "removal", text: old[--x]! }); + } + return edits.reverse(); + } + } + return [ + ...old.map((text): Edit => ({ kind: "removal", text })), + ...next.map((text): Edit => ({ kind: "addition", text })), + ]; +}; diff --git a/packages/diff-viewer/src/prose.ts b/packages/diff-viewer/src/prose.ts index 1dcbaab..452b19d 100644 --- a/packages/diff-viewer/src/prose.ts +++ b/packages/diff-viewer/src/prose.ts @@ -15,6 +15,7 @@ export type DiffTextOptions = Readonly<{ maxSearchCells?: number; }>; +import { myers } from "./myers"; type Edit = Readonly<{ kind: TextDiffPart["kind"]; text: string }>; const tokenize = (text: string, options: DiffTextOptions): string[] => options.granularity === "sentence" @@ -23,58 +24,6 @@ const tokenize = (text: string, options: DiffTextOptions): string[] => ].map((part) => part.segment) : (text.match(/[\p{L}\p{N}\p{M}_]+|\s+|[^\p{L}\p{N}\p{M}_\s]/gu) ?? []); -const myers = (old: readonly string[], next: readonly string[], maxCells: number): Edit[] => { - const trace: Map[] = []; - let frontier = new Map([[1, 0]]); - let cells = 0; - for (let distance = 0; distance <= old.length + next.length; distance++) { - cells += 2 * distance + 1; - if (cells > maxCells) break; - trace.push(new Map(frontier)); - for (let diagonal = -distance; diagonal <= distance; diagonal += 2) { - let x = - diagonal === -distance || - (diagonal !== distance && - (frontier.get(diagonal - 1) ?? -Infinity) < (frontier.get(diagonal + 1) ?? -Infinity)) - ? (frontier.get(diagonal + 1) ?? 0) - : (frontier.get(diagonal - 1) ?? 0) + 1; - let y = x - diagonal; - while (x < old.length && y < next.length && old[x] === next[y]) { - x++; - y++; - } - frontier.set(diagonal, x); - if (x < old.length || y < next.length) continue; - const edits: Edit[] = []; - x = old.length; - y = next.length; - for (let depth = distance; depth >= 0; depth--) { - const previous = trace[depth]!; - const k = x - y; - const previousK = - k === -depth || - (k !== depth && (previous.get(k - 1) ?? -Infinity) < (previous.get(k + 1) ?? -Infinity)) - ? k + 1 - : k - 1; - const previousX = previous.get(previousK) ?? 0; - const previousY = previousX - previousK; - while (x > previousX && y > previousY) { - edits.push({ kind: "equal", text: old[--x]! }); - y--; - } - if (depth === 0) break; - if (x === previousX) edits.push({ kind: "addition", text: next[--y]! }); - else edits.push({ kind: "removal", text: old[--x]! }); - } - return edits.reverse(); - } - } - return [ - ...old.map((text): Edit => ({ kind: "removal", text })), - ...next.map((text): Edit => ({ kind: "addition", text })), - ]; -}; - /** Exact, whitespace-preserving prose diff. No inference of moved sentences. */ export const diffText = ( oldText: string, diff --git a/packages/diff-viewer/src/styles.css b/packages/diff-viewer/src/styles.css index 7f0e7e5..771951c 100644 --- a/packages/diff-viewer/src/styles.css +++ b/packages/diff-viewer/src/styles.css @@ -414,3 +414,52 @@ background: var(--foldworks-ui-danger-surface); text-decoration: line-through; } + +.fk-diff-viewer__widget, +.fk-diff-viewer__annotation { + grid-column: 1 / -1; + padding: 12px; + white-space: normal; +} +.fk-diff-viewer__gutter[data-annotation-tone="danger"] { + box-shadow: inset 3px 0 var(--fk-danger, #e5484d); +} +.fk-diff-viewer__gutter[data-annotation-tone="warning"] { + box-shadow: inset 3px 0 #e5a000; +} +.fk-diff-viewer__gutter[data-annotation-tone="info"] { + box-shadow: inset 3px 0 #3b82f6; +} +.fk-diff-viewer__gutter[data-annotation-tone="success"] { + box-shadow: inset 3px 0 #30a46c; +} +.fk-diff-viewer__code ins { + background: #30a46c33; + text-decoration: none; +} +.fk-diff-viewer__code del { + background: #e5484d33; + text-decoration: line-through; +} +.fk-value-diff { + padding: 16px; + color: var(--foldworks-ui-foreground, inherit); +} +.fk-value-diff ul { + padding: 0; + list-style: none; +} +.fk-value-diff li { + border-block-end: 1px solid var(--foldworks-ui-border, #aaa); + padding-block: 12px; +} +.fk-value-diff pre { + white-space: pre-wrap; + overflow-wrap: anywhere; +} +.fk-value-diff ins { + background: #30a46c22; +} +.fk-value-diff del { + background: #e5484d22; +} diff --git a/packages/diff-viewer/src/values.test.ts b/packages/diff-viewer/src/values.test.ts new file mode 100644 index 0000000..d22d7e8 --- /dev/null +++ b/packages/diff-viewer/src/values.test.ts @@ -0,0 +1,62 @@ +import { expect, it } from "vitest"; +import { inertHtml, type HtmlBuilder } from "foldkit/html"; +import { Scene } from "foldkit/test"; +import { diffValues, ValueDiff, diagnosticsToAnnotations, type JsonValue } from "./values"; +it("compares nested paths, root values and object presence", () => { + expect( + diffValues({ enabled: false, nested: { remove: null } }, { enabled: true, nested: { add: 1 } }), + ).toEqual([ + { path: ["enabled"], kind: "changed", before: false, after: true }, + { path: ["nested", "remove"], kind: "removed", before: null }, + { path: ["nested", "add"], kind: "added", after: 1 }, + ]); + expect(diffValues(null, [1])).toEqual([{ path: [], kind: "changed", before: null, after: [1] }]); +}); +it("matches unique array identities, supports host keys, and falls back to indexes", () => { + expect( + diffValues( + [ + { id: "a", v: 1 }, + { id: "b", v: 2 }, + ], + [ + { id: "b", v: 3 }, + { id: "a", v: 1 }, + ], + ), + ).toEqual([{ path: [0, "v"], kind: "changed", before: 2, after: 3 }]); + expect( + diffValues([{ name: "a" }, { name: "b" }], [{ name: "b" }, { name: "a" }], { + key: (v) => + typeof v === "object" && v !== null && !Array.isArray(v) + ? (v as { name: string }).name + : undefined, + }), + ).toEqual([]); + expect(diffValues([1, 2], [2, 1])).toHaveLength(2); + expect(diffValues([{ id: "a" }, { id: "a" }], [{ id: "b" }])).toHaveLength(2); +}); +it("rejects non-JSON values and cycles", () => { + const cycle: { [key: string]: JsonValue } = {}; + cycle.self = cycle; + expect(() => diffValues(cycle, {})).toThrow("cycles"); + expect(() => diffValues(NaN, 0)).toThrow("JSON-like"); +}); +it("maps UTF-16 offsets, CRLF, exclusive line ends and EOF diagnostics", () => { + const text = "😀a\r\nb\n"; + const annotation = (from: number, to: number) => + diagnosticsToAnnotations("f", text, [{ from, to, severity: "error", message: "Invalid" }])[0]!; + expect(annotation(0, 5)).toMatchObject({ startLine: 1, endLine: 1, tone: "danger" }); + expect(annotation(2, 6)).toMatchObject({ startLine: 1, endLine: 2 }); + expect(annotation(text.length, text.length)).toMatchObject({ startLine: 2, endLine: 2 }); + expect( + diagnosticsToAnnotations("f", "", [{ from: 0, to: 0, severity: "hint", message: "Empty" }])[0]! + .startLine, + ).toBe(1); +}); +it("renders semantic values with semantic insertion and deletion markup", () => { + const h = inertHtml as unknown as HtmlBuilder; + const html = ValueDiff.view({ changes: diffValues({ a: 1 }, { a: 2 }) }, h)!; + expect(Scene.findAll(html, "ins")).toHaveLength(1); + expect(Scene.findAll(html, "del")).toHaveLength(1); +}); diff --git a/packages/diff-viewer/src/values.ts b/packages/diff-viewer/src/values.ts new file mode 100644 index 0000000..a5c0715 --- /dev/null +++ b/packages/diff-viewer/src/values.ts @@ -0,0 +1,192 @@ +import type { Diagnostic } from "@foldworks/text-intelligence"; +import type { Html, HtmlBuilder } from "foldkit/html"; +import type { DiffAnnotation } from "./view"; + +export type JsonValue = + | null + | boolean + | number + | string + | readonly JsonValue[] + | { readonly [key: string]: JsonValue }; +export type ValuePath = readonly (string | number)[]; +export type ValueChange = Readonly<{ + path: ValuePath; + kind: "added" | "removed" | "changed"; + before?: JsonValue; + after?: JsonValue; +}>; +export type DiffValuesOptions = Readonly<{ + key?: (value: JsonValue, path: ValuePath) => string | number | undefined; +}>; +const object = (value: JsonValue): value is { [key: string]: JsonValue } => + value !== null && typeof value === "object" && !Array.isArray(value); +const assertJson = (value: JsonValue, active = new Set()): void => { + if ( + value === null || + typeof value === "string" || + typeof value === "boolean" || + (typeof value === "number" && Number.isFinite(value)) + ) + return; + if ( + typeof value !== "object" || + value === null || + (!Array.isArray(value) && + Object.getPrototypeOf(value) !== Object.prototype && + Object.getPrototypeOf(value) !== null) + ) + throw new TypeError("diffValues requires JSON-like values"); + if (active.has(value)) throw new TypeError("diffValues does not support cycles"); + active.add(value); + for (const item of Array.isArray(value) ? value : Object.values(value)) assertJson(item, active); + active.delete(value); +}; +/** Identity-matched arrays ignore pure reorder; other arrays compare by index. */ +export const diffValues = ( + before: JsonValue, + after: JsonValue, + options: DiffValuesOptions = {}, +): readonly ValueChange[] => { + assertJson(before); + assertJson(after); + const changes: ValueChange[] = []; + const visit = (old: JsonValue, next: JsonValue, path: ValuePath): void => { + if (old === next) return; + if (Array.isArray(old) && Array.isArray(next)) { + const key = + options.key ?? + ((value: JsonValue) => + object(value) && (typeof value.id === "string" || typeof value.id === "number") + ? value.id + : undefined); + const oldKeys = old.map((value) => key(value, path)), + nextKeys = next.map((value) => key(value, path)); + const unique = (keys: readonly (string | number | undefined)[]) => + keys.every((k) => k !== undefined) && new Set(keys).size === keys.length; + if (unique(oldKeys) && unique(nextKeys)) { + const oldByKey = new Map(oldKeys.map((key, index) => [key, index])); + const newByKey = new Set(nextKeys); + old.forEach((value, index) => { + if (!newByKey.has(oldKeys[index])) + changes.push({ path: [...path, index], kind: "removed", before: value }); + }); + next.forEach((value, index) => { + const oldIndex = oldByKey.get(nextKeys[index]); + if (oldIndex === undefined) + changes.push({ path: [...path, index], kind: "added", after: value }); + else visit(old[oldIndex]!, value, [...path, index]); + }); + } else + for (let index = 0; index < Math.max(old.length, next.length); index++) { + if (index >= old.length) + changes.push({ path: [...path, index], kind: "added", after: next[index]! }); + else if (index >= next.length) + changes.push({ path: [...path, index], kind: "removed", before: old[index]! }); + else visit(old[index]!, next[index]!, [...path, index]); + } + } else if (object(old) && object(next)) { + for (const key of new Set([...Object.keys(old), ...Object.keys(next)])) { + if (!Object.hasOwn(old, key)) + changes.push({ path: [...path, key], kind: "added", after: next[key]! }); + else if (!Object.hasOwn(next, key)) + changes.push({ path: [...path, key], kind: "removed", before: old[key]! }); + else visit(old[key]!, next[key]!, [...path, key]); + } + } else changes.push({ path, kind: "changed", before: old, after: next }); + }; + visit(before, after, []); + return changes; +}; +export const valuePathLabel = (path: ValuePath): string => + "$" + + path.map((key) => (typeof key === "number" ? `[${key}]` : `[${JSON.stringify(key)}]`)).join(""); +export const ValueDiff = { + view: ( + config: Readonly<{ + changes: readonly ValueChange[]; + label?: string; + onSelectPath?: (change: ValueChange) => Message; + }>, + h: HtmlBuilder, + ): Html => { + const groups = new Map(); + for (const change of config.changes) { + const group = valuePathLabel(change.path.slice(0, -1)); + groups.set(group, [...(groups.get(group) ?? []), change]); + } + return h.section( + [h.Class("fk-value-diff"), h.AriaLabel(config.label ?? "Value changes")], + [ + ...[...groups].map(([label, changes]) => + h.div( + [], + [ + h.h3([], [label]), + h.ul( + [], + changes.map((change) => + h.li( + [h.DataAttribute("change-kind", change.kind)], + [ + config.onSelectPath + ? h.button( + [h.Type("button"), h.OnClick(config.onSelectPath(change))], + [valuePathLabel(change.path)], + ) + : h.strong([], [valuePathLabel(change.path)]), + h.span([], [` · ${change.kind}`]), + ...(change.kind !== "added" + ? [h.pre([], [h.del([], [JSON.stringify(change.before, null, 2)])])] + : []), + ...(change.kind !== "removed" + ? [h.pre([], [h.ins([], [JSON.stringify(change.after, null, 2)])])] + : []), + ], + ), + ), + ), + ], + ), + ), + ...(config.changes.length ? [] : [h.p([], ["No value changes."])]), + ], + ); + }, +}; +/** Diagnostic offsets address the original UTF-16 text; to is exclusive. */ +export const diagnosticsToAnnotations = ( + path: string, + text: string, + diagnostics: readonly Diagnostic[], +): readonly DiffAnnotation[] => { + const starts = [0]; + for (let offset = 0; offset < text.length; offset++) + if (text[offset] === "\n") starts.push(offset + 1); + const count = Math.max(1, starts.length - (text.endsWith("\n") ? 1 : 0)); + const lineAt = (offset: number) => { + let low = 0, + high = starts.length; + while (low + 1 < high) { + const mid = (low + high) >>> 1; + if (starts[mid]! <= offset) low = mid; + else high = mid; + } + return Math.min(count, low + 1); + }; + return diagnostics + .filter((d) => Number.isFinite(d.from) && Number.isFinite(d.to)) + .map((d) => { + const from = Math.max(0, Math.min(text.length, Math.floor(d.from))), + to = Math.max(from, Math.min(text.length, Math.floor(d.to))); + return { + path, + side: "new", + startLine: lineAt(from), + endLine: lineAt(to > from ? to - 1 : from), + tone: d.severity === "error" ? "danger" : d.severity === "warning" ? "warning" : "info", + label: d.code ?? d.severity, + message: d.message, + }; + }); +}; diff --git a/packages/diff-viewer/src/view.ts b/packages/diff-viewer/src/view.ts index 0825f1f..6090a52 100644 --- a/packages/diff-viewer/src/view.ts +++ b/packages/diff-viewer/src/view.ts @@ -1,10 +1,34 @@ +import { contextGaps, type DiffGap } from "./anchors"; +import type { SemanticToken } from "@foldworks/text-intelligence"; +import { diffText } from "./prose"; import { Option } from "effect"; import type { Html, HtmlBuilder } from "foldkit/html"; -import { adjacentSelectableLine, normalizeSelection, selectionContains, splitRows, type DiffFile, type DiffLine, type DiffSelection, type DiffSelectionRange, type DiffSide, type DiffThreadMarker, type DiffViewMode } from "./core"; +import { + adjacentSelectableLine, + normalizeSelection, + selectionContains, + splitRows, + type DiffFile, + type DiffLine, + type DiffSelection, + type DiffSelectionRange, + type DiffSide, + type DiffThreadMarker, + type DiffViewMode, +} from "./core"; + +export type DiffLineRef = DiffSelection; +export type Highlight = SemanticToken; +export type DiffAnnotation = DiffSelectionRange & + Readonly<{ tone: "info" | "warning" | "danger" | "success"; label: string; message?: string }>; export type ViewConfig = Readonly<{ file: DiffFile; + onExpandGap?: (gap: DiffGap) => Message; + lineWidgets?: (line: DiffLineRef) => Html | undefined; + annotations?: readonly DiffAnnotation[]; + highlight?: (content: string, language: string) => readonly Highlight[]; mode?: DiffViewMode; selectedRange?: DiffSelectionRange; threads?: readonly DiffThreadMarker[]; @@ -17,20 +41,56 @@ export type ViewConfig = Readonly<{ onReviewedChange?: (reviewed: boolean) => Message; }>; -const tokenPattern = /(\/\/.*$|\/\*.*?\*\/|`(?:\\.|[^`])*`|'(?:\\.|[^'])*'|"(?:\\.|[^"])*"|\b(?:const|let|var|function|return|if|else|for|while|type|interface|export|import|from|async|await|new|class|extends|true|false|null|undefined)\b|\b\d+(?:\.\d+)?\b|\b[A-Z][A-Za-z0-9_]*\b)/gm; +const tokenPattern = + /(\/\/.*$|\/\*.*?\*\/|`(?:\\.|[^`])*`|'(?:\\.|[^'])*'|"(?:\\.|[^"])*"|\b(?:const|let|var|function|return|if|else|for|while|type|interface|export|import|from|async|await|new|class|extends|true|false|null|undefined)\b|\b\d+(?:\.\d+)?\b|\b[A-Z][A-Za-z0-9_]*\b)/gm; -const highlighted = (content: string, h: HtmlBuilder): readonly Html[] => { +const highlighted = ( + content: string, + h: HtmlBuilder, + config?: ViewConfig, +): readonly Html[] => { + if (config?.highlight) { + const result: Html[] = []; + let cursor = 0; + for (const token of [...config.highlight(content, config.file.language)].sort( + (a, b) => a.from - b.from, + )) { + if ( + !Number.isInteger(token.from) || + !Number.isInteger(token.to) || + token.from < cursor || + token.to <= token.from || + token.to > content.length + ) + continue; + result.push(h.span([], [content.slice(cursor, token.from)])); + result.push( + h.span( + [h.Class(`fk-diff-viewer__token fk-diff-viewer__token--${token.kind}`)], + [content.slice(token.from, token.to)], + ), + ); + cursor = token.to; + } + result.push(h.span([], [content.slice(cursor)])); + return result; + } const result: Html[] = []; let cursor = 0; for (const match of content.matchAll(tokenPattern)) { const index = match.index; if (index > cursor) result.push(h.span([], [content.slice(cursor, index)])); const value = match[0]; - const kind = value.startsWith("//") || value.startsWith("/*") ? "comment" - : value.startsWith("'") || value.startsWith('"') || value.startsWith("`") ? "string" - : /^\d/.test(value) ? "number" - : /^[A-Z]/.test(value) ? "type" - : "keyword"; + const kind = + value.startsWith("//") || value.startsWith("/*") + ? "comment" + : value.startsWith("'") || value.startsWith('"') || value.startsWith("`") + ? "string" + : /^\d/.test(value) + ? "number" + : /^[A-Z]/.test(value) + ? "type" + : "keyword"; result.push(h.span([h.Class(`fk-diff-viewer__token fk-diff-viewer__token--${kind}`)], [value])); cursor = index + value.length; } @@ -38,11 +98,22 @@ const highlighted = (content: string, h: HtmlBuilder): readonl return result.length === 0 ? [h.span([], [content || " "])] : result; }; -const markerCount = (markers: readonly DiffThreadMarker[], path: string, side: DiffSide, line: number): number => - markers.filter((marker) => { - const range = normalizeSelection(marker); - return range.path === path && range.side === side && range.endLine === line && marker.resolved !== true; - }) +const markerCount = ( + markers: readonly DiffThreadMarker[], + path: string, + side: DiffSide, + line: number, +): number => + markers + .filter((marker) => { + const range = normalizeSelection(marker); + return ( + range.path === path && + range.side === side && + range.endLine === line && + marker.resolved !== true + ); + }) .reduce((sum, marker) => sum + marker.count, 0); const gutter = ( @@ -51,134 +122,359 @@ const gutter = ( line: number | null, h: HtmlBuilder, ): Html => { - if (line === null) return h.span([h.Class("fk-diff-viewer__gutter fk-diff-viewer__gutter--empty")], []); + if (line === null) + return h.span([h.Class("fk-diff-viewer__gutter fk-diff-viewer__gutter--empty")], []); const selection = { path: config.file.path, side, line } as const; const selected = selectionContains(config.selectedRange, selection.path, side, line); const count = markerCount(config.threads ?? [], config.file.path, side, line); - const selectionEnd = config.selectedRange?.path === selection.path && config.selectedRange.side === side - ? config.selectedRange.endLine - : line; - return h.button([ - h.Type("button"), - h.Class("fk-diff-viewer__gutter"), - h.DataAttribute("selected", selected ? "true" : "false"), - h.AriaLabel(`${selected ? "Selected" : "Comment on"} ${side === "old" ? "old" : "new"} line ${line}`), - ...(config.onSelectLine === undefined && config.onStartSelection === undefined - ? [h.Disabled(true)] - : [ - ...(config.onSelectLine === undefined ? [] : [h.OnClick(config.onSelectLine(selection))]), - ...(config.onStartSelection === undefined ? [] : [h.OnMouseDown(config.onStartSelection(selection))]), - ...(config.onExtendSelection === undefined ? [] : [ - h.OnMouseMove(config.onExtendSelection(selection, "Pointer")), - h.OnKeyDownPreventDefault((key, modifiers) => { - if (key === "Escape" && config.onCancelSelection !== undefined) { - return Option.some(config.onCancelSelection()); - } - if (!modifiers.shiftKey || (key !== "ArrowUp" && key !== "ArrowDown")) return Option.none(); - const target = adjacentSelectableLine( - config.file, - side, - selectionEnd, - key === "ArrowUp" ? "Previous" : "Next", - ); - return target === undefined - ? Option.none() - : Option.some(config.onExtendSelection!({ ...selection, line: target }, "Keyboard")); - }), + const selectionEnd = + config.selectedRange?.path === selection.path && config.selectedRange.side === side + ? config.selectedRange.endLine + : line; + const annotations = (config.annotations ?? []).filter((a) => + selectionContains(a, config.file.path, side, line), + ); + return h.button( + [ + h.Id(lineGutterId(selection)), + ...annotations.map((a) => h.Title(`${a.label}${a.message ? `: ${a.message}` : ""}`)), + h.DataAttribute("annotation-tone", annotations[0]?.tone ?? ""), + h.Type("button"), + h.Class("fk-diff-viewer__gutter"), + h.DataAttribute("selected", selected ? "true" : "false"), + h.AriaLabel( + `${selected ? "Selected" : "Comment on"} ${side === "old" ? "old" : "new"} line ${line}${annotations.map((a) => `, ${a.label}`).join("")}`, + ), + ...(config.onSelectLine === undefined && config.onStartSelection === undefined + ? [h.Disabled(true)] + : [ + ...(config.onSelectLine === undefined + ? [] + : [h.OnClick(config.onSelectLine(selection))]), + ...(config.onStartSelection === undefined + ? [] + : [h.OnMouseDown(config.onStartSelection(selection))]), + ...(config.onExtendSelection === undefined + ? [] + : [ + h.OnMouseMove(config.onExtendSelection(selection, "Pointer")), + h.OnKeyDownPreventDefault((key, modifiers) => { + if (key === "Escape" && config.onCancelSelection !== undefined) { + return Option.some(config.onCancelSelection()); + } + if (!modifiers.shiftKey || (key !== "ArrowUp" && key !== "ArrowDown")) + return Option.none(); + const target = adjacentSelectableLine( + config.file, + side, + selectionEnd, + key === "ArrowUp" ? "Previous" : "Next", + ); + return target === undefined + ? Option.none() + : Option.some( + config.onExtendSelection!({ ...selection, line: target }, "Keyboard"), + ); + }), + ]), ]), - ]), - ], [ - h.span([h.Class("fk-diff-viewer__add-comment"), h.AriaHidden(true)], ["+"]), - h.span([h.Class("fk-diff-viewer__line-number")], [String(line)]), - ...(count > 0 ? [h.span([h.Class("fk-diff-viewer__thread-count"), h.AriaLabel(`${count} open comments`)], [String(count)])] : []), - ]); + ], + [ + h.span([h.Class("fk-diff-viewer__add-comment"), h.AriaHidden(true)], ["+"]), + h.span([h.Class("fk-diff-viewer__line-number")], [String(line)]), + ...(count > 0 + ? [ + h.span( + [h.Class("fk-diff-viewer__thread-count"), h.AriaLabel(`${count} open comments`)], + [String(count)], + ), + ] + : []), + ], + ); }; -const unifiedLine = (config: ViewConfig, line: DiffLine, h: HtmlBuilder): Html => { - const side: DiffSide = line.kind === "deletion" ? "old" : "new"; - const number = side === "old" ? line.oldLine : line.newLine; - return h.div([ - h.Class(`fk-diff-viewer__line fk-diff-viewer__line--${line.kind}`), - h.DataAttribute("line-kind", line.kind), - h.DataAttribute("selected", number !== null && selectionContains(config.selectedRange, config.file.path, side, number) ? "true" : "false"), - ], [ - gutter(config, "old", line.oldLine, h), - gutter(config, "new", line.newLine, h), - h.span([h.Class("fk-diff-viewer__prefix"), h.AriaHidden(true)], [line.kind === "addition" ? "+" : line.kind === "deletion" ? "−" : " "]), - h.code([h.Class("fk-diff-viewer__code")], highlighted(line.content, h)), - ...(number === null ? [] : [h.span([h.Class("fk-diff-viewer__sr-only")], [`${side} line ${number}`])]), - ]); +export const lineGutterId = (ref: DiffLineRef): string => + `diff-${encodeURIComponent(ref.path)}-${ref.side}-${ref.line}`; + +const lineContent = ( + config: ViewConfig, + line: DiffLine, + h: HtmlBuilder, + paired?: DiffLine, +): readonly Html[] => { + if (line.kind === "context" || !paired) return highlighted(line.content, h, config); + const tokens = config.highlight?.(line.content, config.file.language); + return diffText( + line.kind === "deletion" ? line.content : paired.content, + line.kind === "addition" ? line.content : paired.content, + ) + .filter((part) => + line.kind === "deletion" ? part.kind !== "addition" : part.kind !== "removal", + ) + .map((part) => { + const from = line.kind === "deletion" ? part.oldFrom : part.newFrom; + const fragmentConfig = tokens + ? { + ...config, + highlight: () => + tokens + .filter((token) => token.to > from && token.from < from + part.text.length) + .map((token) => ({ + ...token, + from: Math.max(0, token.from - from), + to: Math.min(part.text.length, token.to - from), + })), + } + : config; + return part.kind === "equal" + ? h.span([], highlighted(part.text, h, fragmentConfig)) + : part.kind === "addition" + ? h.ins([], [part.text]) + : h.del([], [part.text]); + }); +}; + +const widgets = ( + config: ViewConfig, + line: DiffLine, + h: HtmlBuilder, + side?: DiffSide, +): readonly Html[] => { + const refs = [ + ...(side !== "new" && line.oldLine !== null + ? [{ path: config.file.path, side: "old" as const, line: line.oldLine }] + : []), + ...(side !== "old" && line.newLine !== null + ? [{ path: config.file.path, side: "new" as const, line: line.newLine }] + : []), + ]; + return refs.flatMap((ref) => { + const widget = config.lineWidgets?.(ref); + const annotations = (config.annotations ?? []).filter( + (a) => a.path === ref.path && a.side === ref.side && a.endLine === ref.line, + ); + return [ + ...annotations.map((a) => + h.div( + [h.Class("fk-diff-viewer__annotation"), h.DataAttribute("tone", a.tone)], + [a.label, ...(a.message ? [": " + a.message] : [])], + ), + ), + ...(widget ? [h.div([h.Class("fk-diff-viewer__widget")], [widget])] : []), + ]; + }); }; -const splitCell = (config: ViewConfig, line: DiffLine | undefined, side: DiffSide, h: HtmlBuilder): Html => { - if (line === undefined) return h.div([h.Class("fk-diff-viewer__split-cell fk-diff-viewer__split-cell--empty")], [gutter(config, side, null, h)]); +const unifiedLine = ( + config: ViewConfig, + line: DiffLine, + h: HtmlBuilder, + paired?: DiffLine, +): Html => { + const side: DiffSide = line.kind === "deletion" ? "old" : "new"; const number = side === "old" ? line.oldLine : line.newLine; - return h.div([ - h.Class(`fk-diff-viewer__split-cell fk-diff-viewer__split-cell--${line.kind}`), - h.DataAttribute("selected", number !== null && selectionContains(config.selectedRange, config.file.path, side, number) ? "true" : "false"), - ], [ - gutter(config, side, number, h), - h.span([h.Class("fk-diff-viewer__prefix"), h.AriaHidden(true)], [line.kind === "addition" ? "+" : line.kind === "deletion" ? "−" : " "]), - h.code([h.Class("fk-diff-viewer__code")], highlighted(line.content, h)), - ]); + return h.div( + [ + h.Class(`fk-diff-viewer__line fk-diff-viewer__line--${line.kind}`), + h.DataAttribute("line-kind", line.kind), + h.DataAttribute( + "selected", + number !== null && selectionContains(config.selectedRange, config.file.path, side, number) + ? "true" + : "false", + ), + ], + [ + gutter(config, "old", line.oldLine, h), + gutter(config, "new", line.newLine, h), + h.span( + [h.Class("fk-diff-viewer__prefix"), h.AriaHidden(true)], + [line.kind === "addition" ? "+" : line.kind === "deletion" ? "−" : " "], + ), + h.code([h.Class("fk-diff-viewer__code")], lineContent(config, line, h, paired)), + ...(number === null + ? [] + : [h.span([h.Class("fk-diff-viewer__sr-only")], [`${side} line ${number}`])]), + ], + ); }; -const hiddenLinesBefore = (file: DiffFile, hunkIndex: number): number => { - const current = file.hunks[hunkIndex]; - if (current === undefined) return 0; - if (hunkIndex === 0) return Math.max(0, Math.min(current.oldStart, current.newStart) - 1); - const previous = file.hunks[hunkIndex - 1]!; - return Math.max(0, Math.min( - current.oldStart - (previous.oldStart + previous.oldCount), - current.newStart - (previous.newStart + previous.newCount), - )); +const splitCell = ( + config: ViewConfig, + line: DiffLine | undefined, + side: DiffSide, + h: HtmlBuilder, + paired?: DiffLine, +): Html => { + if (line === undefined) + return h.div( + [h.Class("fk-diff-viewer__split-cell fk-diff-viewer__split-cell--empty")], + [gutter(config, side, null, h)], + ); + const number = side === "old" ? line.oldLine : line.newLine; + return h.div( + [ + h.Class(`fk-diff-viewer__split-cell fk-diff-viewer__split-cell--${line.kind}`), + h.DataAttribute( + "selected", + number !== null && selectionContains(config.selectedRange, config.file.path, side, number) + ? "true" + : "false", + ), + ], + [ + gutter(config, side, number, h), + h.span( + [h.Class("fk-diff-viewer__prefix"), h.AriaHidden(true)], + [line.kind === "addition" ? "+" : line.kind === "deletion" ? "−" : " "], + ), + h.code([h.Class("fk-diff-viewer__code")], lineContent(config, line, h, paired)), + ], + ); }; export const view = (config: ViewConfig, h: HtmlBuilder): Html => { + const pairs = new Map(); + for (const hunk of config.file.hunks) + for (const row of splitRows(hunk)) { + if (row.left?.kind === "deletion" && row.right?.kind === "addition") { + pairs.set(row.left, row.right); + pairs.set(row.right, row.left); + } + } const mode = config.mode ?? "Unified"; const markers = config.threads ?? []; - return h.section([ - h.Class("fk-diff-viewer"), - h.DataAttribute("mode", mode.toLowerCase()), - h.AriaLabel(`Changes in ${config.file.path}`), - ...(config.onEndSelection === undefined ? [] : [ - h.OnMouseUp(config.onEndSelection()), - h.OnMouseLeave(config.onEndSelection()), - ]), - ], [ - h.header([h.Class("fk-diff-viewer__header")], [ - h.div([h.Class("fk-diff-viewer__file")], [ - h.span([h.Class("fk-diff-viewer__status"), h.DataAttribute("status", config.file.status)], [config.file.status === "added" ? "A" : config.file.status === "deleted" ? "D" : config.file.status === "renamed" ? "R" : "M"]), - h.strong([], [config.file.path.split("/").pop() ?? config.file.path]), - h.span([h.Class("fk-diff-viewer__path")], [config.file.path.includes("/") ? config.file.path.slice(0, config.file.path.lastIndexOf("/")) : "Repository root"]), - ]), - h.div([h.Class("fk-diff-viewer__meta")], [ - h.span([h.Class("fk-diff-viewer__additions")], [`+${config.file.additions}`]), - h.span([h.Class("fk-diff-viewer__deletions")], [`−${config.file.deletions}`]), - ...(config.onReviewedChange === undefined ? [] : [h.button([ - h.Type("button"), h.Class("fk-diff-viewer__reviewed"), - h.DataAttribute("checked", config.reviewed === true ? "true" : "false"), - h.OnClick(config.onReviewedChange(config.reviewed !== true)), - h.AriaLabel(`${config.reviewed === true ? "Unmark" : "Mark"} ${config.file.path} as viewed`), - ], [config.reviewed === true ? "✓ Viewed" : "Mark viewed"])]), - ]), - ]), - ...(config.file.status === "binary" - ? [h.div([h.Class("fk-diff-viewer__empty")], ["Binary file changed. Preview is not available."])] - : config.file.hunks.flatMap((hunk, hunkIndex) => { - const hidden = hiddenLinesBefore(config.file, hunkIndex); - const gap = hidden > 0 ? [h.div([h.Class("fk-diff-viewer__gap")], [`⋯ ${hidden} unchanged ${hidden === 1 ? "line" : "lines"}`])] : []; - const header = h.div([h.Class("fk-diff-viewer__hunk-header")], [ - h.code([], [hunk.header.replace(/\s+@@.*$/, " @@")]), - h.span([], [hunk.label || "Changed block"]), - ]); - const content = mode === "Unified" - ? hunk.lines.map((line) => unifiedLine({ ...config, threads: markers }, line, h)) - : splitRows(hunk).map((row) => h.div([h.Class("fk-diff-viewer__split-row")], [ - splitCell({ ...config, threads: markers }, row.left, "old", h), - splitCell({ ...config, threads: markers }, row.right, "new", h), - ])); - return [...gap, header, ...content]; - })), - ]); + return h.section( + [ + h.Class("fk-diff-viewer"), + h.DataAttribute("mode", mode.toLowerCase()), + h.AriaLabel(`Changes in ${config.file.path}`), + ...(config.onEndSelection === undefined + ? [] + : [h.OnMouseUp(config.onEndSelection()), h.OnMouseLeave(config.onEndSelection())]), + ], + [ + h.header( + [h.Class("fk-diff-viewer__header")], + [ + h.div( + [h.Class("fk-diff-viewer__file")], + [ + h.span( + [h.Class("fk-diff-viewer__status"), h.DataAttribute("status", config.file.status)], + [ + config.file.status === "added" + ? "A" + : config.file.status === "deleted" + ? "D" + : config.file.status === "renamed" + ? "R" + : "M", + ], + ), + h.strong([], [config.file.path.split("/").pop() ?? config.file.path]), + h.span( + [h.Class("fk-diff-viewer__path")], + [ + config.file.path.includes("/") + ? config.file.path.slice(0, config.file.path.lastIndexOf("/")) + : "Repository root", + ], + ), + ], + ), + h.div( + [h.Class("fk-diff-viewer__meta")], + [ + h.span([h.Class("fk-diff-viewer__additions")], [`+${config.file.additions}`]), + h.span([h.Class("fk-diff-viewer__deletions")], [`−${config.file.deletions}`]), + ...(config.onReviewedChange === undefined + ? [] + : [ + h.button( + [ + h.Type("button"), + h.Class("fk-diff-viewer__reviewed"), + h.DataAttribute("checked", config.reviewed === true ? "true" : "false"), + h.OnClick(config.onReviewedChange(config.reviewed !== true)), + h.AriaLabel( + `${config.reviewed === true ? "Unmark" : "Mark"} ${config.file.path} as viewed`, + ), + ], + [config.reviewed === true ? "✓ Viewed" : "Mark viewed"], + ), + ]), + ], + ), + ], + ), + ...(config.file.status === "binary" + ? [ + h.div( + [h.Class("fk-diff-viewer__empty")], + ["Binary file changed. Preview is not available."], + ), + ] + : config.file.hunks.flatMap((hunk) => { + const expandable = contextGaps(config.file).find((g) => g.beforeHunkId === hunk.id); + const hidden = expandable?.count ?? 0; + const gap = + hidden > 0 + ? [ + h.div( + [h.Class("fk-diff-viewer__gap")], + [ + ...(config.onExpandGap && expandable + ? [ + h.button( + [h.Type("button"), h.OnClick(config.onExpandGap(expandable))], + [`Expand ${hidden} unchanged lines`], + ), + ] + : [`⋯ ${hidden} unchanged ${hidden === 1 ? "line" : "lines"}`]), + ], + ), + ] + : []; + const header = h.div( + [h.Class("fk-diff-viewer__hunk-header")], + [ + h.code([], [hunk.header.replace(/\s+@@.*$/, " @@")]), + h.span([], [hunk.label || "Changed block"]), + ], + ); + const content = + mode === "Unified" + ? hunk.lines.flatMap((line) => [ + unifiedLine({ ...config, threads: markers }, line, h, pairs.get(line)), + ...widgets(config, line, h), + ]) + : splitRows(hunk).flatMap((row) => [ + h.div( + [h.Class("fk-diff-viewer__split-row")], + [ + splitCell( + { ...config, threads: markers }, + row.left, + "old", + h, + row.left ? pairs.get(row.left) : undefined, + ), + splitCell( + { ...config, threads: markers }, + row.right, + "new", + h, + row.right ? pairs.get(row.right) : undefined, + ), + ], + ), + ...(row.left ? widgets(config, row.left, h, "old") : []), + ...(row.right ? widgets(config, row.right, h, "new") : []), + ]); + return [...gap, header, ...content]; + })), + ], + ); }; diff --git a/packages/review/CHANGELOG.md b/packages/review/CHANGELOG.md new file mode 100644 index 0000000..ab91c12 --- /dev/null +++ b/packages/review/CHANGELOG.md @@ -0,0 +1 @@ +# @foldworks/review diff --git a/packages/review/LICENSE b/packages/review/LICENSE new file mode 100644 index 0000000..ac737c0 --- /dev/null +++ b/packages/review/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Ben Jacobson + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/packages/review/README.md b/packages/review/README.md new file mode 100644 index 0000000..3b7008d --- /dev/null +++ b/packages/review/README.md @@ -0,0 +1,117 @@ +# @foldworks/review + +Provider-neutral review UI for Foldkit. The host owns transport, persistence, +identity, permissions, policy, and merging. Import `@foldworks/review/styles.css` +once for the package CSS. Suggestion previews also need +`@foldworks/diff-viewer/styles.css`. This package uses the same Effect and Foldkit peers as +the other Foldworks primitives. + +## Threads and drafts + +`Comment` is an Effect schema. `Thread(AnchorSchema)` builds a schema for any +host anchor; `Thread` is its TypeScript shape. Thread data stays in the +host. Pass it to `ReviewThreads.threadView({ model, thread, toMessage }, h)`. +The default body renderer preserves plain text. Supply `renderBody(comment)` +for Markdown, mentions or links, and `canEdit`/`canDelete` for visible actions. +The host must enforce permissions for every request. + +Initialize `ReviewThreads.init({ id })`; fold its `update` with +`Update.foldChild`. It returns `RequestedCreateThread`, `RequestedReply`, +`RequestedEdit`, `RequestedDelete`, `RequestedResolve`, `RequestedReopen`, or +`RequestedActivate` out-messages. These express intent and never mutate host data. +New anchors use a host `anchorKey`, so the submodel is independent of anchor +shape. Use stable, unique draft keys per anchor, reply or edit. + +`OpenedComposer({ draftKey, returnFocusId })` opens and focuses a composer. +`composerView({ model, draftKey, targetId, kind, toMessage }, h)` renders it. +Mod+Enter submits, Escape closes and returns focus. Closing retains drafts. +After saving, send `Committed({ draftKey, body })` with the exact saved body; +only a matching draft clears. A failed save needs no acknowledgement. A delayed +acknowledgement cannot clear newer typing. Hosts should serialize concurrent +saves per draft key and assign their own request IDs if needed. + +`listView` groups threads using `groupBy`, supports all/open/resolved/outdated +filters, and has one list tab stop with Arrow/Home/End movement and Enter to +reveal the active thread. Hosts provide anchor display state; resolved threads +collapse until expanded. Request and save status use a polite live region. +Focus targets must still exist when a command executes. Thread IDs, UI IDs and +draft keys must be stable and unique within the surface. + +## Outdated threads + +Pass `anchorState: 'current' | 'moved' | 'outdated'` to `threadView` and an +`anchorState(thread)` function to `listView`. These are derived host display +inputs, never persisted in `Thread`. For diff anchors use `mapRange` from +`@foldworks/diff-viewer`; retain the original anchor and source revision when +mapping reports outdated, so discussion stays available in the list even if +its line no longer exists. + +## Suggestions and pending reviews + +A `Comment` may carry a `Suggestion` with path, inclusive 1-based start/end +lines, replacement and **expected** original content. Capture expected content +when proposing it, joined with LF without the range's terminal newline. +`applySuggestions(text, suggestions)` returns `applied` with new text, or +`rejected` with `stale`, `overlap`, or `mixed-paths`. A rejection changes nothing; +stale results identify invalid ranges and mismatching expected text. It never +searches for similar content. The host verifies the path, commits the returned +text, remaps anchors and resolves threads. + +Application normalizes CRLF to LF, preserves the source's final newline +convention and treats empty replacement as deletion of complete lines. A single +terminal newline on replacement is ignored. Zero-width insertions are not +supported; replace an existing range with the desired complete lines instead. +`threadView` previews the suggestion as a diff; `onApplySuggestion` exposes the +host action. Omit the callback when applying is unavailable. + +`pending` on comments is host data. `ReviewSubmit.view` takes controlled summary, +verdict, pending count, allowed verdicts and submission callbacks. Unsupported +verdicts cannot be selected; submission disables for a disallowed current +choice or `isSubmitting`. Hosts must also validate submission intent and retain +drafts on failure. `ReviewerList.view` displays requested/returned verdicts, +required labels, and optional ownership counts. Neither view evaluates policy. + +## Change-request composition + +`ChangedFiles.init({ id, files })` and `ChangedFiles.update(model, message, +{ files, mode })` wrap the UI `Tree` submodel. Pass the same file inputs to +`ChangedFiles.view({ model, files, mode, toMessage }, h)`. The host handles +`SelectedFile` and `RequestedViewed` out-messages. Directories aggregate additions, +deletions and open threads. Status letters and annotation tones have text +labels. The filter and unviewed toggle affect display, never host file data. +`mode: 'flat'` removes directories. Tree Arrow/Home/End keys navigate; Space +toggles the active file's viewed checkbox. Paths must be unique. Initial files +expand all directories; hosts can reconcile expansion for newly arriving paths. + +`CheckList.view` summarizes queued/running/passed/failed/skipped/cancelled/neutral +checks, with optional `onRerun(check)` and native expandable `renderDetails(check)`. +Duration is a host-formatted string; details links and their access policy belong +to the host. No network requests or execution happen in the view. + +`MergeReadiness.view` takes host-evaluated requirements (including nested +requirements and optional actions), `canMerge`, `onMerge`, and optional controlled +strategy choices. Nested requirements reuse `ExplanationTree`. `canMerge` is +authoritative; the view never derives authorization from requirement labels. +The host must check policy again when processing the merge message. Strategies +use arbitrary host IDs and labels. + +`ChangeRequestHeader.view` composes `AppHeader` and `Badge` with title, status, +author, base/head labels and host action Html. Use UI `TransactionTimeline` +entries with `kind` and `renderBody(entry)` for one ordered activity stream. + +## Suggestion composers and reference host + +Pass a `suggestionAnchor` (path, inclusive lines and expected text) to +`composerView` to offer an optional replacement editor. `ReviewThreads` owns +these replacement drafts alongside comment bodies. Acknowledgements for saves +with suggestions include the saved `suggestion`, so typing a new replacement +while saving is protected too. `readOnly` on `threadView` hides mutation actions +while retaining resolved-thread expansion and body rendering. + +The `/diff-viewer` reference app composes this package with CodeEditor, +DiffViewer, ValueDiff, ChangeSetPreview and TransactionTimeline. It is a synthetic +browser-local configuration workflow. Its illustrative host policy requires +schema and rollout checks, approval of the current revision, publication of +pending comments, and resolution of current discussions. Outdated discussion is +retained without blocking this example's merge. Identity switching, validation, +persistence and the simulated merge are demo host code, not package services. diff --git a/packages/review/package.json b/packages/review/package.json new file mode 100644 index 0000000..ecaaeac --- /dev/null +++ b/packages/review/package.json @@ -0,0 +1,63 @@ +{ + "name": "@foldworks/review", + "version": "0.1.0", + "description": "Provider-neutral review threads and change-request composition for Foldkit.", + "license": "MIT", + "author": "Ben Jacobson", + "type": "module", + "files": [ + "dist", + "README.md", + "LICENSE" + ], + "sideEffects": [ + "**/*.css" + ], + "repository": { + "type": "git", + "url": "git+https://github.com/bjacobso/foldworks.git", + "directory": "packages/review" + }, + "homepage": "https://github.com/bjacobso/foldworks#readme", + "bugs": { + "url": "https://github.com/bjacobso/foldworks/issues" + }, + "keywords": [ + "foldkit", + "review", + "comments", + "accessibility" + ], + "publishConfig": { + "access": "public", + "registry": "https://registry.npmjs.org/" + }, + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js" + }, + "./styles.css": "./dist/styles.css" + }, + "scripts": { + "build": "node ../../scripts/build-package.mjs", + "build:runtime": "node ../../scripts/build-package.mjs --runtime-only", + "prepack": "pnpm run build", + "test": "vitest run", + "typecheck": "tsc --noEmit" + }, + "devDependencies": { + "effect": "catalog:", + "foldkit": "catalog:", + "typescript": "catalog:", + "vitest": "catalog:" + }, + "peerDependencies": { + "effect": "catalog:peers", + "foldkit": "catalog:peers" + }, + "dependencies": { + "@foldworks/diff-viewer": "workspace:*", + "@foldworks/ui": "workspace:*" + } +} diff --git a/packages/review/src/chrome.test.ts b/packages/review/src/chrome.test.ts new file mode 100644 index 0000000..684122e --- /dev/null +++ b/packages/review/src/chrome.test.ts @@ -0,0 +1,81 @@ +import { expect, it } from "vitest"; +import { inertHtml, type HtmlBuilder } from "foldkit/html"; +import { Scene } from "foldkit/test"; +import { CheckList, checkSummary, MergeReadiness, ChangeRequestHeader } from "./chrome"; +import * as Files from "./files"; +const h = inertHtml as unknown as HtmlBuilder; +const files = [ + { + path: "config/a.json", + status: "modified" as const, + additions: 2, + deletions: 1, + openThreads: 2, + }, + { path: "config/b.json", status: "added" as const, additions: 4, deletions: 0, viewed: true }, +]; +it("aggregates directory counts and filters viewed files without owning them", () => { + const model = Files.init({ id: "files", files }); + expect(Files.fileNodes(files)[0]!.label).toContain("+6 −1 · 2 open threads"); + expect(Files.visibleFiles({ ...model, onlyUnviewed: true }, { files })).toHaveLength(1); + expect(Files.fileNodes(files, "flat")).toHaveLength(2); + expect( + Files.update(model, Files.Message.RequestedViewed({ path: files[0]!.path, viewed: true }), { + files, + }).outMessage, + ).toMatchObject({ _tag: "RequestedViewed", viewed: true }); + const html = Files.view({ model, files, toMessage: () => "files" }, h)!; + expect(Scene.findAll(html, '[role="tree"]')).toHaveLength(1); + expect(Scene.findAll(html, 'input[type="checkbox"]')).toHaveLength(3); +}); +it("summarizes all check statuses and provides optional details and rerun intent", () => { + const checks = [ + { id: "s", name: "Schema", status: "failed" as const }, + { id: "p", name: "Policy", status: "passed" as const }, + ]; + expect(checkSummary(checks)).toBe("1 failed, 1 passed"); + const html = CheckList.view( + { checks, onRerun: () => "rerun", renderDetails: () => h.p([], ["Local validator"]) }, + h, + )!; + expect(Scene.findAll(html, "details")).toHaveLength(2); + expect(Scene.findAll(html, "button")).toHaveLength(2); +}); +it("uses host merge availability even when displayed requirements differ", () => { + const html = MergeReadiness.view( + { + requirements: [ + { + id: "r", + label: "Approval", + outcome: "met", + reason: "Reviewed", + children: [{ id: "c", label: "Owner", outcome: "met", reason: "Confirmed" }], + }, + ], + canMerge: false, + onMerge: "merge", + strategies: [{ id: "custom", label: "Apply configuration" }], + strategy: "custom", + onStrategyChange: () => "strategy", + }, + h, + )!; + expect(Scene.findAll(html, "button[disabled]")).toHaveLength(1); + expect(Scene.findAll(html, "select")).toHaveLength(1); + expect( + Scene.findAll( + ChangeRequestHeader.view( + { + title: "Enable flag", + status: "draft", + author: { id: "a", name: "Author" }, + baseLabel: "saved", + headLabel: "draft", + }, + h, + )!, + "header", + ), + ).toHaveLength(1); +}); diff --git a/packages/review/src/chrome.ts b/packages/review/src/chrome.ts new file mode 100644 index 0000000..3478ff1 --- /dev/null +++ b/packages/review/src/chrome.ts @@ -0,0 +1,167 @@ +import { Schema as S } from "effect"; +import type { Html, HtmlBuilder } from "foldkit/html"; +import { AppHeader, Badge, ExplanationTree, type ExplanationNode } from "@foldworks/ui"; +import type { Author } from "./data"; + +export const Check = S.Struct({ + id: S.String, + name: S.String, + status: S.Literals(["queued", "running", "passed", "failed", "skipped", "cancelled", "neutral"]), + summary: S.optional(S.String), + duration: S.optional(S.String), + detailsHref: S.optional(S.String), +}); +export type Check = typeof Check.Type; +export const checkSummary = (checks: readonly Check[]): string => + checks.length + ? (["failed", "running", "queued", "passed", "skipped", "cancelled", "neutral"] as const) + .flatMap((status) => { + const count = checks.filter((c) => c.status === status).length; + return count ? [`${count} ${status}`] : []; + }) + .join(", ") + : "No checks"; +export const CheckList = { + view: ( + config: Readonly<{ + checks: readonly Check[]; + onRerun?: (check: Check) => Message; + renderDetails?: (check: Check) => Html | undefined; + }>, + h: HtmlBuilder, + ): Html => + h.section( + [h.Class("fk-review__panel"), h.AriaLabel("Checks")], + [ + h.h3([], ["Checks"]), + h.p([h.Role("status"), h.AriaLive("polite")], [checkSummary(config.checks)]), + h.ul( + [], + config.checks.map((check) => { + const details = config.renderDetails?.(check); + return h.li( + [h.DataAttribute("check-status", check.status)], + [ + h.strong([], [check.name]), + h.span([], [` · ${check.status}`]), + ...(check.duration ? [h.span([], [` · ${check.duration}`])] : []), + ...(check.summary ? [h.p([], [check.summary])] : []), + ...(check.detailsHref ? [h.a([h.Href(check.detailsHref)], ["Check details"])] : []), + ...(details ? [h.details([], [h.summary([], ["Show details"]), details])] : []), + ...(config.onRerun + ? [ + h.button( + [ + h.Type("button"), + h.OnClick(config.onRerun(check)), + h.AriaLabel(`Re-run ${check.name}`), + ], + ["Re-run"], + ), + ] + : []), + ], + ); + }), + ), + ], + ), +}; +export type MergeRequirement = Readonly<{ + id: string; + label: string; + outcome: "met" | "unmet" | "pending" | "bypassable"; + reason: string; + action?: Readonly<{ label: string; message: Message }>; + children?: readonly MergeRequirement[]; +}>; +export type MergeStrategy = Readonly<{ id: string; label: string }>; +export const MergeReadiness = { + view: ( + config: Readonly<{ + requirements: readonly MergeRequirement[]; + canMerge: boolean; + onMerge: Message; + strategies?: readonly MergeStrategy[]; + strategy?: string; + onStrategyChange?: (strategy: string) => Message; + }>, + h: HtmlBuilder, + ): Html => { + const nodes = ( + requirements: readonly MergeRequirement[], + ): readonly ExplanationNode[] => + requirements.map((r) => ({ + label: `${r.label} · ${r.outcome}`, + detail: r.reason, + outcome: r.outcome === "met" ? "passed" : r.outcome === "unmet" ? "failed" : "unknown", + ...(r.children ? { children: nodes(r.children) } : {}), + })); + const actions = (requirements: readonly MergeRequirement[]): readonly Html[] => + requirements.flatMap((r) => [ + ...(r.action + ? [h.button([h.Type("button"), h.OnClick(r.action.message)], [r.action.label])] + : []), + ...actions(r.children ?? []), + ]); + return h.section( + [h.Class("fk-review__panel"), h.AriaLabel("Merge readiness")], + [ + h.h3([], ["Merge readiness"]), + ExplanationTree.view({ label: "Merge requirements", nodes: nodes(config.requirements) }, h), + ...actions(config.requirements), + ...(config.strategies?.length && config.onStrategyChange + ? [ + h.select( + [ + h.AriaLabel("Merge strategy"), + h.Value(config.strategy ?? ""), + h.OnChange(config.onStrategyChange), + ], + config.strategies.map((strategy) => + h.option([h.Value(strategy.id)], [strategy.label]), + ), + ), + ] + : []), + h.button( + [h.Type("button"), h.Disabled(!config.canMerge), h.OnClick(config.onMerge)], + ["Merge change request"], + ), + ], + ); + }, +}; +export type ChangeRequestStatus = "draft" | "open" | "merged" | "closed"; +export const ChangeRequestHeader = { + view: ( + config: Readonly<{ + title: string; + status: ChangeRequestStatus; + author: Author; + baseLabel: string; + headLabel: string; + actions?: readonly Html[]; + }>, + h: HtmlBuilder, + ): Html => + h.div( + [h.Class("fk-review__header")], + [ + AppHeader.view( + { + title: config.title, + status: [ + Badge.view( + { label: config.status, tone: config.status === "merged" ? "info" : "neutral" }, + h, + ), + ], + ...(config.actions ? { actions: config.actions } : {}), + }, + h, + ), + h.p([], [`${config.author.name} proposes ${config.headLabel} → ${config.baseLabel}`]), + ], + ), +}; diff --git a/packages/review/src/data.ts b/packages/review/src/data.ts new file mode 100644 index 0000000..a8d54c1 --- /dev/null +++ b/packages/review/src/data.ts @@ -0,0 +1,29 @@ +import { Suggestion } from "./suggestions"; +import { Schema as S } from "effect"; + +export const Author = S.Struct({ id: S.String, name: S.String, avatarUrl: S.optional(S.String) }); +export type Author = typeof Author.Type; +export const Comment = S.Struct({ + suggestion: S.optional(Suggestion), + id: S.String, + author: Author, + body: S.String, + createdAt: S.String, + editedAt: S.optional(S.String), + pending: S.optional(S.Boolean), +}); +export type Comment = typeof Comment.Type; +export const Thread = (Anchor: Anchor) => + S.Struct({ + id: S.String, + anchor: Anchor, + status: S.Literals(["open", "resolved"]), + comments: S.Array(Comment), + }); +export type Thread = Readonly<{ + id: string; + anchor: Anchor; + status: "open" | "resolved"; + comments: readonly Comment[]; +}>; +export type AnchorState = "current" | "moved" | "outdated"; diff --git a/packages/review/src/files.ts b/packages/review/src/files.ts new file mode 100644 index 0000000..e63b264 --- /dev/null +++ b/packages/review/src/files.ts @@ -0,0 +1,165 @@ +import { Option, Schema as S } from "effect"; +import { Update } from "foldkit"; +import { defineMessageUnion } from "foldkit/message"; +import type { Html, HtmlBuilder } from "foldkit/html"; +import { Tree } from "@foldworks/ui"; +import type { DiffFileStatus } from "@foldworks/diff-viewer"; + +export type ChangedFile = Readonly<{ + path: string; + status: DiffFileStatus; + additions: number; + deletions: number; + viewed?: boolean; + openThreads?: number; + annotationTone?: "info" | "warning" | "danger" | "success"; +}>; +export type Config = Readonly<{ files: readonly ChangedFile[]; mode?: "tree" | "flat" }>; +export const Model = S.Struct({ tree: Tree.Model, filter: S.String, onlyUnviewed: S.Boolean }); +export type Model = typeof Model.Type; +export const Message = defineMessageUnion({ + GotTreeMessage: { message: Tree.Message }, + ChangedFilter: { value: S.String }, + ChangedOnlyUnviewed: { value: S.Boolean }, + RequestedViewed: { path: S.String, viewed: S.Boolean }, +}); +export type Message = typeof Message.Type; +export const OutMessage = defineMessageUnion({ + SelectedFile: { path: S.String }, + RequestedViewed: { path: S.String, viewed: S.Boolean }, +}); +export type OutMessage = typeof OutMessage.Type; +const fileId = (path: string) => `file:${path}`; +export const visibleFiles = (model: Model, config: Config): readonly ChangedFile[] => + config.files.filter( + (file) => + (!model.onlyUnviewed || !file.viewed) && + file.path.toLocaleLowerCase().includes(model.filter.trim().toLocaleLowerCase()), + ); +export const fileNodes = ( + files: readonly ChangedFile[], + mode: "tree" | "flat" = "tree", +): readonly Tree.Node[] => { + if (new Set(files.map((f) => f.path)).size !== files.length) + throw new Error("Changed file paths must be unique"); + const nodes: Tree.Node[] = []; + const directories = new Set(); + const stats = (subset: readonly ChangedFile[]) => + `+${subset.reduce((sum, f) => sum + f.additions, 0)} −${subset.reduce((sum, f) => sum + f.deletions, 0)} · ${subset.reduce((sum, f) => sum + (f.openThreads ?? 0), 0)} open threads`; + for (const file of files) { + const parts = file.path.split("/"); + let parentId: string | null = null; + if (mode === "tree") + for (let i = 1; i < parts.length; i++) { + const prefix = parts.slice(0, i).join("/"); + const id = `dir:${prefix}`; + if (!directories.has(id)) { + nodes.push({ + id, + parentId, + label: `${parts[i - 1]} · ${stats(files.filter((f) => f.path.startsWith(prefix + "/")))}`, + branch: true, + renamable: false, + movable: false, + }); + directories.add(id); + } + parentId = id; + } + nodes.push({ + id: fileId(file.path), + parentId, + label: `${file.status[0]!.toUpperCase()} ${mode === "flat" ? file.path : parts.at(-1)} · ${stats([file])}${file.annotationTone ? ` · ${file.annotationTone} annotation` : ""}`, + branch: false, + renamable: false, + movable: false, + }); + } + return nodes; +}; +export const init = (config: Readonly<{ id: string; files?: readonly ChangedFile[] }>): Model => ({ + tree: Tree.init({ + id: config.id, + expandedIds: fileNodes(config.files ?? []) + .filter((n) => n.branch) + .map((n) => n.id), + }), + filter: "", + onlyUnviewed: false, +}); +export const update = ( + model: Model, + message: Message, + config: Config, +): Update.ReturnWithOutMessage => + Message.match(message, { + ChangedFilter: ({ value }) => ({ model: { ...model, filter: value } }), + ChangedOnlyUnviewed: ({ value }) => ({ model: { ...model, onlyUnviewed: value } }), + RequestedViewed: ({ path, viewed }) => + config.files.some((f) => f.path === path) + ? { + model: { ...model, tree: { ...model.tree, activeId: fileId(path) } }, + outMessage: OutMessage.RequestedViewed({ path, viewed }), + } + : { model }, + GotTreeMessage: ({ message }) => { + return Update.foldChild({ + update: (tree: Tree.Model, message: Tree.Message) => + Tree.update(tree, message, { + nodes: fileNodes(visibleFiles(model, config), config.mode), + }), + read: (model: Model) => Option.some(model.tree), + write: (model, tree) => ({ ...model, tree }), + toParentMessage: (message) => Message.GotTreeMessage({ message }), + foldOutMessage: (out) => (model: Model) => + out._tag === "Selected" && out.id.startsWith("file:") + ? { model, outMessage: OutMessage.SelectedFile({ path: out.id.slice(5) }) } + : { model }, + })(model, message); + }, + }); +export const view = ( + config: Config & Readonly<{ model: Model; toMessage: (message: Message) => ParentMessage }>, + h: HtmlBuilder, +): Html => { + const files = visibleFiles(config.model, config), + { toMessage } = config; + return h.section( + [h.Class("fk-review__panel"), h.AriaLabel("Changed files")], + [ + h.h3([], ["Changed files"]), + h.input([ + h.Type("search"), + h.AriaLabel("Filter changed files"), + h.Value(config.model.filter), + h.OnInput((value) => toMessage(Message.ChangedFilter({ value }))), + ]), + h.label( + [], + [ + h.input([ + h.Type("checkbox"), + h.Checked(config.model.onlyUnviewed), + h.OnClick( + toMessage(Message.ChangedOnlyUnviewed({ value: !config.model.onlyUnviewed })), + ), + ]), + "Show only unviewed", + ], + ), + Tree.view( + { + model: config.model.tree, + nodes: fileNodes(files, config.mode), + label: "Changed files", + showActions: false, + checkedIds: files.filter((f) => f.viewed).map((f) => fileId(f.path)), + onCheckedChange: (id, viewed) => + toMessage(Message.RequestedViewed({ path: id.slice(5), viewed })), + toParentMessage: (message) => toMessage(Message.GotTreeMessage({ message })), + }, + h, + ), + ], + ); +}; diff --git a/packages/review/src/index.ts b/packages/review/src/index.ts new file mode 100644 index 0000000..bdc4ead --- /dev/null +++ b/packages/review/src/index.ts @@ -0,0 +1,6 @@ +export * from "./data"; +export * as ReviewThreads from "./threads"; +export * from "./suggestions"; +export * from "./submission"; +export * as ChangedFiles from "./files"; +export * from "./chrome"; diff --git a/packages/review/src/styles.css b/packages/review/src/styles.css new file mode 100644 index 0000000..94180cd --- /dev/null +++ b/packages/review/src/styles.css @@ -0,0 +1,69 @@ +.fk-review__thread, +.fk-review__composer, +.fk-review__list { + border: 1px solid var(--foldworks-ui-border, #aaa); + border-radius: 8px; + padding: 12px; + margin-block: 8px; + color: var(--foldworks-ui-foreground, inherit); + background: var(--foldworks-ui-surface, transparent); +} +.fk-review__comment { + border-bottom: 1px solid var(--foldworks-ui-border, #aaa); + padding-block: 12px; +} +.fk-review__comment time { + margin-inline: 8px; + font-size: 0.8em; +} +.fk-review__body { + white-space: pre-wrap; + overflow-wrap: anywhere; +} +.fk-review__composer textarea { + display: block; + width: 100%; + min-height: 80px; + background: inherit; + color: inherit; + font: inherit; +} +.fk-review__thread button, +.fk-review__composer button, +.fk-review__list button { + margin: 4px; +} +.fk-review__list [role="option"][aria-selected="true"] { + outline: 2px solid currentColor; +} +.fk-review__sr { + position: absolute; + width: 1px; + height: 1px; + overflow: hidden; + clip-path: inset(50%); +} +.fk-review__panel { + border: 1px solid var(--foldworks-ui-border, #aaa); + padding: 12px; + border-radius: 8px; + margin-block: 8px; +} +.fk-review__panel textarea { + display: block; + width: 100%; + color: inherit; + background: inherit; +} +.fk-review__panel button { + margin: 4px; +} +.fk-review__panel [data-verdict="approve"] { + color: var(--foldworks-ui-success, #198359); +} +.fk-review__panel [data-verdict="request-changes"] { + color: var(--foldworks-ui-danger, #d04444); +} +.fk-review__panel [data-verdict="requested"] { + opacity: 0.7; +} diff --git a/packages/review/src/submission.test.ts b/packages/review/src/submission.test.ts new file mode 100644 index 0000000..58c8f5c --- /dev/null +++ b/packages/review/src/submission.test.ts @@ -0,0 +1,68 @@ +import { expect, it } from "vitest"; +import { inertHtml, type HtmlBuilder } from "foldkit/html"; +import { Scene } from "foldkit/test"; +import { ReviewSubmit, ReviewerList } from "./submission"; +import { threadView, init } from "./threads"; +const h = inertHtml as unknown as HtmlBuilder; +it("restricts verdicts and disables submission for a disallowed choice", () => { + const html = ReviewSubmit.view( + { + summary: "", + verdict: "approve", + pendingCount: 2, + allowedVerdicts: ["comment"], + onSummaryChange: () => "summary", + onVerdictChange: () => "verdict", + onSubmit: "submit", + }, + h, + )!; + expect(Scene.findAll(html, "button")).toHaveLength(2); + expect(Scene.findAll(html, "button[disabled]")).toHaveLength(1); + expect( + Scene.findAll( + ReviewerList.view( + { + reviewers: [ + { + author: { id: "r", name: "Reviewer" }, + required: true, + ownedFileCount: 2, + verdict: "request-changes", + }, + ], + }, + h, + )!, + '[data-verdict="request-changes"]', + ), + ).toHaveLength(1); +}); +it("shows pending suggestions and exposes application only when supplied by the host", () => { + const thread = { + id: "t", + anchor: 1, + status: "open" as const, + comments: [ + { + id: "c", + author: { id: "r", name: "R" }, + createdAt: "today", + body: "Use this", + pending: true, + suggestion: { path: "f", startLine: 1, endLine: 1, expected: "a", replacement: "b" }, + }, + ], + }; + const html = threadView( + { + model: init({ id: "t" }), + thread, + toMessage: () => "message", + onApplySuggestion: () => "apply", + }, + h, + )!; + expect(Scene.findAll(html, "ins")).toHaveLength(1); + expect(Scene.findAll(html, ".fk-diff-viewer")).toHaveLength(1); +}); diff --git a/packages/review/src/submission.ts b/packages/review/src/submission.ts new file mode 100644 index 0000000..999b307 --- /dev/null +++ b/packages/review/src/submission.ts @@ -0,0 +1,94 @@ +import { Schema as S } from "effect"; +import type { Html, HtmlBuilder } from "foldkit/html"; +import type { Author } from "./data"; + +export const Verdict = S.Literals(["comment", "approve", "request-changes"]); +export type Verdict = typeof Verdict.Type; +export type ReviewSubmitConfig = Readonly<{ + summary: string; + verdict: Verdict; + pendingCount: number; + allowedVerdicts: readonly Verdict[]; + onSummaryChange: (summary: string) => Message; + onVerdictChange: (verdict: Verdict) => Message; + onSubmit: Message; + isSubmitting?: boolean; +}>; +export const ReviewSubmit = { + view: (config: ReviewSubmitConfig, h: HtmlBuilder): Html => + h.form( + [h.Class("fk-review__panel"), h.AriaLabel("Submit review"), h.OnSubmit(config.onSubmit)], + [ + h.h3([], ["Submit review"]), + h.p([h.Role("status")], [`${config.pendingCount} pending comments`]), + h.textarea([ + h.AriaLabel("Review summary"), + h.Value(config.summary), + h.OnInput(config.onSummaryChange), + ]), + h.div( + [h.Role("group"), h.AriaLabel("Review verdict")], + config.allowedVerdicts.map((verdict) => + h.button( + [ + h.Type("button"), + h.AriaPressed(String(config.verdict === verdict)), + h.Disabled(config.isSubmitting === true), + h.OnClick(config.onVerdictChange(verdict)), + ], + [ + verdict === "request-changes" + ? "Request changes" + : verdict === "approve" + ? "Approve" + : "Comment", + ], + ), + ), + ), + h.button( + [ + h.Type("submit"), + h.Disabled( + config.isSubmitting === true || !config.allowedVerdicts.includes(config.verdict), + ), + ], + [config.isSubmitting ? "Submitting…" : "Submit review"], + ), + ], + ), +}; +export type Reviewer = Readonly<{ + author: Author; + verdict?: Verdict; + required?: boolean; + ownedFileCount?: number; +}>; +export const ReviewerList = { + view: ( + config: Readonly<{ reviewers: readonly Reviewer[]; label?: string }>, + h: HtmlBuilder, + ): Html => + h.section( + [h.Class("fk-review__panel"), h.AriaLabel(config.label ?? "Reviewers")], + [ + h.h3([], [config.label ?? "Reviewers"]), + h.ul( + [], + config.reviewers.map((reviewer) => + h.li( + [h.DataAttribute("verdict", reviewer.verdict ?? "requested")], + [ + h.strong([], [reviewer.author.name]), + h.span([], [` · ${reviewer.verdict ?? "requested"}`]), + ...(reviewer.required ? [h.span([], [" · required"])] : []), + ...(reviewer.ownedFileCount !== undefined + ? [h.span([], [` · owner of ${reviewer.ownedFileCount} files`])] + : []), + ], + ), + ), + ), + ], + ), +}; diff --git a/packages/review/src/suggestions.test.ts b/packages/review/src/suggestions.test.ts new file mode 100644 index 0000000..c5d4bcc --- /dev/null +++ b/packages/review/src/suggestions.test.ts @@ -0,0 +1,49 @@ +import { expect, it } from "vitest"; +import { applySuggestions, type Suggestion } from "./suggestions"; +const suggestion = ( + startLine: number, + endLine: number, + expected: string, + replacement: string, +): Suggestion => ({ path: "f", startLine, endLine, expected, replacement }); +it("applies a batch bottom-up with multiline replacements and deletions", () => { + expect( + applySuggestions("a\nb\nc\nd\n", [ + suggestion(1, 1, "a", "A\nextra"), + suggestion(3, 4, "c\nd", ""), + ]), + ).toEqual({ status: "applied", text: "A\nextra\nb\n" }); + expect(applySuggestions("a\r\nb", [suggestion(2, 2, "b", "B")])).toEqual({ + status: "applied", + text: "a\nB", + }); + expect(applySuggestions("a\n", [suggestion(1, 1, "a", "")])).toEqual({ + status: "applied", + text: "", + }); +}); +it("rejects stale, overlapping, invalid and mixed-file batches atomically", () => { + expect(applySuggestions("a\nb\n", [suggestion(1, 1, "old", "A")])).toMatchObject({ + status: "rejected", + reason: "stale", + stale: [{ expected: "old" }], + }); + expect( + applySuggestions("a\nb\n", [suggestion(1, 2, "a\nb", "A"), suggestion(2, 2, "b", "B")]), + ).toMatchObject({ status: "rejected", reason: "overlap" }); + expect(applySuggestions("a", [suggestion(0, 1, "a", "A")])).toMatchObject({ reason: "stale" }); + expect( + applySuggestions("a", [ + suggestion(1, 1, "a", "A"), + { ...suggestion(1, 1, "a", "A"), path: "g" }, + ]), + ).toMatchObject({ reason: "mixed-paths" }); + expect(applySuggestions("a\r\n", [])).toEqual({ status: "applied", text: "a\r\n" }); +}); + +it("distinguishes replacing a line with a blank line from deleting it", () => { + expect(applySuggestions("a\nb\n", [suggestion(1, 1, "a", "\n")])).toEqual({ + status: "applied", + text: "\nb\n", + }); +}); diff --git a/packages/review/src/suggestions.ts b/packages/review/src/suggestions.ts new file mode 100644 index 0000000..0be2a61 --- /dev/null +++ b/packages/review/src/suggestions.ts @@ -0,0 +1,56 @@ +import { Schema as S } from "effect"; + +export const Suggestion = S.Struct({ + path: S.String, + startLine: S.Number, + endLine: S.Number, + replacement: S.String, + /** Exact original range content, joined with LF, without its terminal newline. */ + expected: S.String, +}); +export type Suggestion = typeof Suggestion.Type; +export type SuggestionResult = + | Readonly<{ status: "applied"; text: string }> + | Readonly<{ + status: "rejected"; + reason: "overlap" | "mixed-paths" | "stale"; + stale: readonly Suggestion[]; + }>; +/** Atomic application; the host chooses when to commit and resolve threads. */ +export const applySuggestions = ( + text: string, + suggestions: readonly Suggestion[], +): SuggestionResult => { + if (!suggestions.length) return { status: "applied", text }; + const normalized = text.replace(/\r\n/g, "\n"); + const finalNewline = normalized.endsWith("\n"); + const lines = + normalized === "" ? [] : normalized.slice(0, finalNewline ? -1 : undefined).split("\n"); + const sorted = [...suggestions].sort((a, b) => a.startLine - b.startLine); + if (new Set(sorted.map((s) => s.path)).size > 1) + return { status: "rejected", reason: "mixed-paths", stale: [] }; + const stale = sorted.filter( + (s) => + !Number.isInteger(s.startLine) || + !Number.isInteger(s.endLine) || + s.startLine < 1 || + s.endLine < s.startLine || + s.endLine > lines.length || + lines.slice(s.startLine - 1, s.endLine).join("\n") !== s.expected, + ); + if (stale.length) return { status: "rejected", reason: "stale", stale }; + if (sorted.some((s, i) => i > 0 && s.startLine <= sorted[i - 1]!.endLine)) + return { status: "rejected", reason: "overlap", stale: [] }; + for (const suggestion of sorted.reverse()) { + const replacement = suggestion.replacement.replace(/\r\n/g, "\n").replace(/\n$/, ""); + lines.splice( + suggestion.startLine - 1, + suggestion.endLine - suggestion.startLine + 1, + ...(suggestion.replacement === "" ? [] : replacement.split("\n")), + ); + } + return { + status: "applied", + text: lines.length ? lines.join("\n") + (finalNewline ? "\n" : "") : "", + }; +}; diff --git a/packages/review/src/threads.test.ts b/packages/review/src/threads.test.ts new file mode 100644 index 0000000..61119d7 --- /dev/null +++ b/packages/review/src/threads.test.ts @@ -0,0 +1,165 @@ +import { Schema as S } from "effect"; +import { inertHtml, type HtmlBuilder } from "foldkit/html"; +import { Scene } from "foldkit/test"; +import { expect, it } from "vitest"; +import { Comment, Thread } from "./data"; +import * as Threads from "./threads"; + +const initial = Threads.init({ id: "review" }); +const comment = { + id: "c", + author: { id: "u", name: "Reviewer" }, + body: "Plain