Accept a change repeated across stories once, instead of story by story

cli/src/diff.ts

@@ -1,5 +1,6 @@
 import type { RawImage } from "selfie-store";
 import type { BoundingBox, ChangedHunk, PixelDiff, HunkKind } from "selfie-ui/app";
+import { fade, seamOf } from "selfie-ui/diff-image";
 import { explainHunks, forEachUnexplained } from "./explain.ts";
 import { encodePng } from "./png.ts";
 import { findHunks } from "./hunks.ts";
@@ -9,18 +10,6 @@ export type { BoundingBox, PixelDiff } from "selfie-ui/app";
 type Color = readonly [number, number, number];
 
 const COLORS: Readonly<Record<HunkKind, Color>> = { changed: [255, 0, 80], moved: [9, 105, 218], spacing: [255, 214, 102] };
-// An unchanged pixel fades into gray from this level to white, so the page stays readable
-// behind the highlight and no color in it can be mistaken for one.
-const FADED_FLOOR = 190;
-
-function fade(from: Uint8Array, out: Uint8Array, at: number): void {
-  const luma = (from[at]! * 77 + from[at + 1]! * 150 + from[at + 2]! * 29) >> 8;
-  const faded = FADED_FLOOR + ((luma * (255 - FADED_FLOOR)) >> 8);
-  out[at] = faded;
-  out[at + 1] = faded;
-  out[at + 2] = faded;
-}
-
 function paint(out: Uint8Array, at: number, [red, green, blue]: Color): void {
   out[at] = red;
   out[at + 1] = green;
@@ -94,12 +83,10 @@ export function diffImages(current: RawImage, golden: RawImage): { readonly diff
     changedPixels += explanation.changedPixels;
     if (explanation.changedPixels === goldenOnly && moved.length === 0 && (kind !== "spacing" || goldenSpacingRows > 0)) {
       // Nothing here is different, since something was only taken away, so mark the seam it left.
-      const seam = Math.min(here.y, height - 1);
-      const from = here.width === 0 ? 0 : here.x;
-      const to = here.width === 0 ? width : here.x + here.width;
-      for (let x = from; x < to; x++) {
-        if (kind === "changed") mark(x, seam);
-        paint(out, (seam * width + x) * 3, COLORS[kind]);
+      const seam = seamOf(here, current);
+      for (let x = seam.x; x < seam.x + seam.width; x++) {
+        if (kind === "changed") mark(x, seam.y);
+        paint(out, (seam.y * width + x) * 3, COLORS[kind]);
       }
     }
     return { ...hunk, kind, changedPixels: explanation.changedPixels, moved };

cli/test/deltas.test.ts

@@ -0,0 +1,88 @@
+// What the page relies on: the differ's hunks account for everything, so redrawing them onto the
+// golden gives exactly what's left, with no need to compare any pixels again.
+import type { RawImage } from "selfie-store";
+import { canApply, patchGolden } from "selfie-ui/deltas";
+import { describe, expect, test } from "vitest";
+import { diffImages } from "../src/diff.ts";
+
+const WIDTH = 120;
+
+/** Every row its own, as with real text, so rows line up across images. */
+function page(rows: readonly (readonly [number, number, number])[]): RawImage {
+  const pixels = new Uint8Array(WIDTH * rows.length * 3);
+  rows.forEach((row, y) => {
+    for (let x = 0; x < WIDTH; x++) pixels.set([(row[0] + x * (y % 5)) & 255, row[1], row[2]], (y * WIDTH + x) * 3);
+  });
+  return { width: WIDTH, height: rows.length, pixels };
+}
+
+const rowsOf = (from: number, count: number) => Array.from({ length: count }, (_, at): readonly [number, number, number] => [(from + at) & 255, ((from + at) * 7) & 255, ((from + at) >> 8) & 255]);
+
+const blank = (count: number) => Array.from({ length: count }, (): readonly [number, number, number] => [255, 255, 255]);
+
+function redrawn(current: RawImage, golden: RawImage, accepted: readonly number[]) {
+  const { diff } = diffImages(current, golden);
+  const patch = patchGolden(diff, accepted);
+  const { width, height } = patch.diff.golden;
+  const pixels = new Uint8Array(width * height * 3);
+  for (const { from, box, toX, toY } of patch.blits) {
+    const source = from === "golden" ? golden : current;
+    for (let row = 0; row < box.height; row++) {
+      const start = ((box.y + row) * source.width + box.x) * 3;
+      pixels.set(source.pixels.subarray(start, start + box.width * 3), ((toY + row) * width + toX) * 3);
+    }
+  }
+  return { diff, patch, image: { width, height, pixels } satisfies RawImage };
+}
+
+/** A header that grows, a change further down, and a band below that closes up, as one real change did. */
+const golden = page([...rowsOf(0, 8), ...rowsOf(1000, 20), ...rowsOf(100, 60), ...rowsOf(200, 4), ...rowsOf(300, 60), ...blank(15), ...rowsOf(400, 40)]);
+const current = page([...rowsOf(0, 8), ...rowsOf(2000, 35), ...rowsOf(100, 60), ...rowsOf(600, 4), ...rowsOf(300, 60), ...rowsOf(400, 40)]);
+
+function narrowed(image: RawImage): RawImage {
+  const pixels = image.pixels.slice();
+  for (let y = 30; y < 34; y++) for (let x = 40; x < 44; x++) pixels.set([1, 2, 3], (y * image.width + x) * 3);
+  return { ...image, pixels };
+}
+
+describe("redrawing hunks onto the golden", () => {
+  test("every hunk here can be applied, and there's more than one", () => {
+    const { diff } = diffImages(current, golden);
+    expect(diff.hunks.length).toBeGreaterThan(1);
+    expect(diff.hunks.every((hunk) => canApply(hunk, diff))).toBe(true);
+  });
+
+  test("all of them give the current image, pixel for pixel", () => {
+    const hunks = diffImages(current, golden).diff.hunks.map((_, index) => index);
+    const { image, patch } = redrawn(current, golden, hunks);
+    expect(patch.diff.hunks).toEqual([]);
+    expect({ width: image.width, height: image.height }).toEqual({ width: current.width, height: current.height });
+    expect(Buffer.compare(image.pixels, current.pixels)).toBe(0);
+  });
+
+  test("any of them, in any combination, leave exactly the others to find", () => {
+    const { hunks } = diffImages(current, golden).diff;
+    for (let mask = 0; mask < 1 << hunks.length; mask++) {
+      const accepted = hunks.flatMap((_, index) => ((mask >> index) & 1 ? [index] : []));
+      const { image, patch } = redrawn(current, golden, accepted);
+      const again = diffImages(current, image).diff;
+      expect(again.hunks.map(({ current: box }) => box)).toEqual(patch.diff.hunks.map(({ current: box }) => box));
+      expect(again.hunks.map(({ golden: box }) => box)).toEqual(patch.diff.hunks.map(({ golden: box }) => box));
+    }
+  });
+
+  test("a narrow hunk of one size goes where the golden has it, below one that grew", () => {
+    const changed = narrowed(current);
+    const { hunks } = diffImages(changed, golden).diff;
+    const { image } = redrawn(changed, golden, hunks.map((_, index) => index));
+    expect(Buffer.compare(image.pixels, changed.pixels)).toBe(0);
+  });
+
+  test("a change in width replaces the whole image", () => {
+    const wider = { width: 100, height: 4, pixels: new Uint8Array(100 * 4 * 3).fill(9) };
+    const narrower = { width: 80, height: 4, pixels: new Uint8Array(80 * 4 * 3).fill(200) };
+    const { image } = redrawn(narrower, wider, [0]);
+    expect(Buffer.compare(image.pixels, narrower.pixels)).toBe(0);
+    expect(image.width).toBe(80);
+  });
+});

cli/test/ui-access.test.ts

@@ -84,7 +84,7 @@ describe("who may ask", () => {
       const refused = await get("/api/stories", { host: "evil.example" });
       for (const response of [await get("/"), await get("/api/stories"), refused]) {
         expect(response.headers.get("content-security-policy")).toBe(
-          "default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; frame-ancestors 'none'; base-uri 'none'; form-action 'none'",
+          "default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob:; frame-ancestors 'none'; base-uri 'none'; form-action 'none'",
         );
         expect(response.headers.get("x-frame-options")).toBe("DENY");
         expect(response.headers.get("cross-origin-resource-policy")).toBe("same-origin");

docs/specs/access.md

@@ -14,7 +14,7 @@ There's no login on this server, since anyone who can reach loopback on the mach
 - **Must listen only on loopback and check the `Host` header**, so a website the user visits can't read their renders through it, by pointing its own DNS name at `127.0.0.1` for one (rationale). The header must be `127.0.0.1:<port>` or `localhost:<port>`, with the port it is listening on, and anything else gets 403.
 - **Must refuse a request whose `Origin` isn't this server's**, whatever its method.
 - **Must require `Content-Type: application/json` and the header `X-Selfie-Request: review-ui` on anything but GET and HEAD**, so a form or a page on another site can't approve anything, and must send no CORS headers, so a page's own attempt fails its preflight (rationale).
-- **Must refuse to be framed and to be embedded**, with `frame-ancestors 'none'`, `X-Frame-Options: DENY`, and `Cross-Origin-Resource-Policy: same-origin`, so another site can't steer a click on Approve (rationale), on every response, refusals included. `Content-Security-Policy` is exactly `default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; frame-ancestors 'none'; base-uri 'none'; form-action 'none'`, with `frame-src` naming the hosted Storybook's origin if there is one, through `hono/secure-headers`.
+- **Must refuse to be framed and to be embedded**, with `frame-ancestors 'none'`, `X-Frame-Options: DENY`, and `Cross-Origin-Resource-Policy: same-origin`, so another site can't steer a click on Approve (rationale), on every response, refusals included. `Content-Security-Policy` is exactly `default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob:; frame-ancestors 'none'; base-uri 'none'; form-action 'none'`, with `frame-src` naming the hosted Storybook's origin if there is one, through `hono/secure-headers`.
 - **Must host a Storybook on a loopback port of its own, read only, never on the review server's origin**, since its code is whatever branch was built, and on that origin it could approve its own images (rationale). The `Host` check applies to it too. It may be framed only by itself, since Storybook frames its own preview, and by the review page, and it serves nothing from a directory with no `iframe.html`. Kept built Storybooks share that origin (`docs/specs/ui.md` → Routes).
 - **Must let the review page frame only the hosted Storybook's origin, and sandbox every frame, without `allow-top-navigation`**, so a story can't steer the review page, and can't read it, being another origin (rationale).
 - **Must serve an image only through the app, by a validated hash, with `Cache-Control: private`**, and never name a file path in a response. Reserved: [Serving images](#serving-images) holds the same for the cloud.

docs/specs/glossary.md

@@ -11,6 +11,7 @@
 | **Environment** | What decides the pixels apart from the project's own input: the runner, its architecture, the browser and its flags, the viewport and scale, and the render recipe (`docs/specs/system.md`). Its ID is part of a render's name. |
 | **Render batch** | The record of the process that rendered a set of images, against a Git sha. `batch` for short. |
 | **Reference** | What a story is compared with at a commit: the image of the newest approval that commit's history reaches (`docs/specs/renders.md` → Approvals). A story with none has no reference. Code and the page still say "golden" for it, as in `goldenHash`, until that's renamed. |
+| **Delta** | One change repeated across stories: hunks whose two sides are the same size and hold the same pixels (`docs/specs/ui.md` → Screens). |
 | **Baseline** | What `selfie review --baseline` compares with, for one review: a batch, or a commit's history. |
 | **Note** | What a person says about a story, for an agent to act on: on an image, and maybe a box of it (`docs/specs/feedback.md`). |
 | **Session** | Whose notes these are: a local Git worktree's top folder (`docs/specs/feedback.md`). |

docs/specs/ui.md

@@ -13,6 +13,8 @@ Source of truth: `ui/`.
 |---|---|
 | `ui/src/shared/api.ts` | The JSON on the wire, and `ReviewSource` |
 | `ui/src/shared/hunks.ts`, exported as `selfie-ui/hunks` | Grouping a diff's hunks for showing, for the client and the CLI's document |
+| `ui/src/shared/diff-image.ts`, exported as `selfie-ui/diff-image` | How a diff image draws what didn't change and where something was taken away, for the differ and a page redrawing one |
+| `ui/src/shared/deltas.ts`, exported as `selfie-ui/deltas` | What a [delta](#screens) is, which to offer, and what redrawing one leaves; tests in `ui/test/deltas.test.ts`, and in `cli/test/deltas.test.ts` against the real differ |
 | `ui/src/shared/notes.ts`, exported as `selfie-ui/notes` | Shared note validation and open-note state (`docs/specs/feedback.md`) |
 | `ui/src/shared/renderProgress.ts`, exported as `selfie-ui/render-progress` | A render's estimate and how it's said, for the client and `selfie flakes`; tests in `ui/test/renderProgress.test.ts` |
 | `selfie-ui/sync`, `selfie-ui/sync-api` | `docs/specs/sync.md` → Routes |
@@ -126,7 +128,7 @@ Source of truth: `ui/src/client/`.
 
 - **Must say a story is renamed if it's added and a removed story's reference is its very image**, one to one, and the closest names first where several share an image (rationale). The list has it once, under its new name. This is only to look at: to decisions they're still one added and one removed.
 
-**The batch** shows, in order: its commit, batch ID, whether its pixels were rendered earlier and by which batch, dirty state, environment, and decision actor; a warning if it was recorded dirty; the counts; blockers; and the differences.
+**The batch** shows, in order: any [repeated changes](#screens); its commit, batch ID, whether its pixels were rendered earlier and by which batch, dirty state, environment, and decision actor; a warning if it was recorded dirty; the counts; blockers; and the differences.
 
 - **Must group blockers by reason, in words, and offer each as a filter of the list.**
 - **Must keep the filter, text and state, in the URL, and nowhere else**, so back, reload, and next and previous all follow it.
@@ -137,6 +139,20 @@ Source of truth: `ui/src/client/`.
 - **Must confirm every bulk decision in a dialog, with the count, how many are changed and added, how many are already decided, and which stories.** Focus starts on Cancel.
 - **Must hold the rows in the dialog from when it opens, and send those**, so what it listed is what's sent, whatever a filter or a read does meanwhile.
 
+**Repeated changes:** a delta is one change repeated across stories (`docs/specs/glossary.md`). Source of truth: `ui/src/shared/deltas.ts`, `ui/src/client/deltas.ts`, `ui/src/client/DeltaOffers.tsx`.
+
+- **Must keep all of this in the browser.** It adds no route and no table, and sends only approvals, through the same route any decision takes. Which deltas are accepted is held in `localStorage`, by batch, so a reload keeps them, and another browser, batch, or person doesn't have them (rationale).
+- **Must look for deltas only for someone who [may decide](#screens)**, since nothing else can come of it, and looking decodes every changed image.
+- **Must find a delta from a pair's own diff, fetched by both hashes, and never a review's stats** ([Screens](#screens)), since those are by story, and could be of another pair.
+- **Must group raw hunks, not the groups a list shows, by the size of each side, and then by the SHA-256 of each side's pixels**, wherever in an image a hunk sits (rationale). It decodes only images with a size at least 3 stories share.
+- **Must only group a hunk that can be redrawn on its own:** one with the same size on both sides, or one the whole width of both images. A narrower hunk that changes height only balances out with others in its column, so it stays a hunk to look at.
+- **Must offer each delta at least 3 undecided changed stories have**, most stories first, once all of them have been looked through, each with how many stories and a crop of it before and after. An image with a decision isn't affected.
+- **Must let someone go through a delta's stories before accepting it**, one delta at a time, starting with the first and moving on once it's accepted: previous and next through its stories, wrapping around, naming each with a link to it, and showing that story's crop and all of its image on each side as it is on record, with the delta outlined where that story has it (rationale).
+- **Must say what's accepted, and let each be undone**, which brings its hunks back and takes back no approval.
+- **Must approve an image once every one of its hunks is an accepted delta, and only an undecided one**, so a rejection is never overturned, and confirm that as any [bulk decision](#screens) is, with accepting nothing if it's cancelled. A delta that finishes no image needs no confirming, since it decides nothing.
+- **Must redraw only on a batch's own pages.** One render's golden and diff are drawn from a pair's images and the server's diff image, with its pixel stats, hunk list, and outlines all from the hunks left, and it says that it's redrawn, that the golden on record hasn't changed, and offers that golden instead (rationale).
+- **Must keep what's on the golden to a note on all of it while it's redrawn**, since a box on it is in pixels the golden on record doesn't have in that place. Its existing boxes aren't shown then.
+
 **One render** shows its state and decision, pixel stats, and images.
 
 - **Must offer the images side by side and stacked whenever a story has more than one to look at**, as an added story does with a note on an older image. Stacked, switching layers costs no scroll, and the zoom starts at actual size: the image's width divided by `batch.scale`, in CSS pixels, so it's the size the page really was on every display.

docs/specs/ui.rationale.md

@@ -28,6 +28,14 @@ Evidence and reasoning for `docs/specs/ui.md`, keyed by its headings.
 
 ## Screens
 
+**Repeated changes, in the browser.** A header that grew 15 rows made all 124 stories of one batch change, and each had to be looked at for whatever else was in it. Of 146 distinct hunks there, 28 were in at least 3 stories, and those accounted for all of 83 images. Whether accepting one should outlive a batch, or be something a team shares, is what **Scope: hunks** in `docs/specs/renders.md` is for. Nothing here narrows that, since a store only ever hears of an approval of a whole image.
+
+**Why raw hunks, and by pixels.** A group is only a choice of what to show together, and its box holds whatever's between its hunks, so the same change could group differently beside something else. Position isn't compared, since a band that closes up sits at a different height on each page. A hash and not pixels held in memory, since a whole page's change on each of 568 stories would hold gigabytes. SHA-256, since this approves images, and a weaker hash would let a crafted image pass as a change someone accepted.
+
+**Why whole images to go through a delta's stories.** A delta's pixels are the same in every story by definition, so its crop can't tell one from another. What can be wrong is where it landed and what's around it, as when a change that's right on most pages is on top of something on one. Only one is open at a time, since 28 pairs of whole images would be a very long page.
+
+**Why it needs no second comparison.** Everything outside a hunk matches, which is what makes hunks correct at all, so with some redrawn, what's different is exactly the rest. `cli/test/deltas.test.ts` shows it on the real differ, by comparing again for every combination. It was also compared pixel by pixel in a browser, on all 83 of that batch's finished images, through canvas and PNG.
+
 - **Why moves and spacing are a filter, not a pass.** On one real layout shift, 59 of 134 stories had nothing else in them. Looking through them one at a time is the cost the label should save, and `kind` must not decide for anyone (`cli.rationale.md`), so the filter lets a person see just those, glance over the thumbnails, and approve what's shown.
 - **Why a list even for one hunk.** A whole-image approval is the case of one hunk, so the screen shouldn't have two shapes, and a person learns one way of looking.
 - **Why a flip holds the hunk still.** Once something above a hunk has changed height, the hunk is at a different place in each image, and a flip shows the page jump and nothing about the hunk. The 9-slice already knows which boxes correspond, so the scroll follows them. It's only while looking at a hunk, since flipping with none costs no scroll.