test: add focused coverage for review metrics (#133)

Author: benvinegar

src/core/loaders.ts

@@ -242,7 +242,7 @@ function buildUntrackedDiffFile(
 }
 
 /** Reorder files to follow agent-context narrative order when a sidecar provides one. */
-function orderDiffFiles(files: DiffFile[], agentContext: AgentContext | null) {
+export function orderDiffFiles(files: DiffFile[], agentContext: AgentContext | null) {
   if (!agentContext || agentContext.files.length === 0) {
     return files;
   }

test/fixtures/diff-helpers.ts

@@ -0,0 +1,63 @@
+import { parseDiffFromFile } from "@pierre/diffs";
+import type { DiffFile } from "../../src/core/types";
+
+export function lines(...values: string[]) {
+  return `${values.join("\n")}\n`;
+}
+
+export function createDiffFile({
+  after = "const alpha = 10;\nconst beta = 2;\nconst gamma = 30;\nconst stable = true;\n",
+  before = "const alpha = 1;\nconst beta = 2;\nconst gamma = 3;\nconst stable = true;\n",
+  id = "example",
+  language = "typescript",
+  path = "example.ts",
+  previousPath,
+}: {
+  after?: string;
+  before?: string;
+  id?: string;
+  language?: string;
+  path?: string;
+  previousPath?: string;
+} = {}): DiffFile {
+  const metadata = parseDiffFromFile(
+    { cacheKey: `${id}:before`, contents: before, name: path },
+    { cacheKey: `${id}:after`, contents: after, name: path },
+    { context: 0 },
+    true,
+  );
+
+  return {
+    agent: null,
+    id,
+    language,
+    metadata,
+    patch: "",
+    path,
+    previousPath,
+    stats: { additions: 1, deletions: 1 },
+  };
+}
+
+export function createHeaderOnlyDiffFile(): DiffFile {
+  const file = createDiffFile({
+    before: "const alpha = 1;\n",
+    after: "const alpha = 2;\n",
+    id: "header-only",
+    path: "header-only.ts",
+  });
+
+  return {
+    ...file,
+    metadata: {
+      ...file.metadata,
+      isPartial: true,
+      hunks: file.metadata.hunks.map((hunk) => ({
+        ...hunk,
+        additionLines: 0,
+        deletionLines: 0,
+        hunkContent: [],
+      })),
+    },
+  };
+}

test/hunk-scroll.test.ts

@@ -0,0 +1,59 @@
+import { describe, expect, test } from "bun:test";
+import { computeHunkRevealScrollTop } from "../src/ui/lib/hunkScroll";
+
+describe("computeHunkRevealScrollTop", () => {
+  test("keeps a fitting hunk fully visible when the preferred padding would clip the end", () => {
+    expect(
+      computeHunkRevealScrollTop({
+        hunkTop: 20,
+        hunkHeight: 10,
+        preferredTopPadding: 4,
+        viewportHeight: 12,
+      }),
+    ).toBe(18);
+  });
+
+  test("preserves the preferred top padding when the full hunk still fits", () => {
+    expect(
+      computeHunkRevealScrollTop({
+        hunkTop: 20,
+        hunkHeight: 10,
+        preferredTopPadding: 4,
+        viewportHeight: 16,
+      }),
+    ).toBe(16);
+  });
+
+  test("biases toward the hunk top when the hunk is taller than the viewport", () => {
+    expect(
+      computeHunkRevealScrollTop({
+        hunkTop: 40,
+        hunkHeight: 18,
+        preferredTopPadding: 5,
+        viewportHeight: 10,
+      }),
+    ).toBe(35);
+  });
+
+  test("clamps negative tops and padding at the start of the content", () => {
+    expect(
+      computeHunkRevealScrollTop({
+        hunkTop: -3,
+        hunkHeight: 6,
+        preferredTopPadding: 4,
+        viewportHeight: 12,
+      }),
+    ).toBe(0);
+  });
+
+  test("falls back to the desired top when the viewport height is zero", () => {
+    expect(
+      computeHunkRevealScrollTop({
+        hunkTop: 25,
+        hunkHeight: 8,
+        preferredTopPadding: 6,
+        viewportHeight: 0,
+      }),
+    ).toBe(19);
+  });
+});

test/loaders-ordering.test.ts

@@ -0,0 +1,68 @@
+import { describe, expect, test } from "bun:test";
+import type { AgentContext } from "../src/core/types";
+import { orderDiffFiles } from "../src/core/loaders";
+import { createDiffFile } from "./fixtures/diff-helpers";
+
+function agentContext(...paths: string[]): AgentContext {
+  return {
+    files: paths.map((path) => ({ annotations: [], path })),
+    version: 1,
+  };
+}
+
+describe("orderDiffFiles", () => {
+  test("orders files by agent narrative order", () => {
+    const files = [
+      createDiffFile({ id: "beta", path: "beta.ts" }),
+      createDiffFile({ id: "gamma", path: "gamma.ts" }),
+      createDiffFile({ id: "alpha", path: "alpha.ts" }),
+    ];
+
+    const ordered = orderDiffFiles(files, agentContext("alpha.ts", "gamma.ts"));
+
+    expect(ordered.map((file) => file.path)).toEqual(["alpha.ts", "gamma.ts", "beta.ts"]);
+  });
+
+  test("keeps files not mentioned in context in their original relative order at the end", () => {
+    const files = [
+      createDiffFile({ id: "beta", path: "beta.ts" }),
+      createDiffFile({ id: "delta", path: "delta.ts" }),
+      createDiffFile({ id: "alpha", path: "alpha.ts" }),
+      createDiffFile({ id: "gamma", path: "gamma.ts" }),
+    ];
+
+    const ordered = orderDiffFiles(files, agentContext("gamma.ts"));
+
+    expect(ordered.map((file) => file.path)).toEqual([
+      "gamma.ts",
+      "beta.ts",
+      "delta.ts",
+      "alpha.ts",
+    ]);
+  });
+
+  test("returns files unchanged when the agent context is empty or missing", () => {
+    const files = [
+      createDiffFile({ id: "alpha", path: "alpha.ts" }),
+      createDiffFile({ id: "beta", path: "beta.ts" }),
+    ];
+
+    expect(orderDiffFiles(files, null).map((file) => file.path)).toEqual(["alpha.ts", "beta.ts"]);
+    expect(orderDiffFiles(files, agentContext()).map((file) => file.path)).toEqual([
+      "alpha.ts",
+      "beta.ts",
+    ]);
+  });
+
+  test("matches context entries by previousPath when a file was renamed", () => {
+    const files = [
+      createDiffFile({ id: "renamed", path: "new-name.ts", previousPath: "old-name.ts" }),
+      createDiffFile({ id: "beta", path: "beta.ts" }),
+      createDiffFile({ id: "alpha", path: "alpha.ts" }),
+    ];
+
+    const ordered = orderDiffFiles(files, agentContext("alpha.ts", "old-name.ts"));
+
+    expect(ordered.map((file) => file.path)).toEqual(["alpha.ts", "new-name.ts", "beta.ts"]);
+  });
+});

test/section-heights.test.ts

@@ -0,0 +1,117 @@
+import { describe, expect, test } from "bun:test";
+import type { VisibleAgentNote } from "../src/ui/lib/agentAnnotations";
+import { measureDiffSectionMetrics } from "../src/ui/lib/sectionHeights";
+import { resolveTheme } from "../src/ui/themes";
+import { createDiffFile, createHeaderOnlyDiffFile, lines } from "./fixtures/diff-helpers";
+
+describe("measureDiffSectionMetrics", () => {
+  const theme = resolveTheme("midnight", null);
+
+  test("measures split and stack layouts from the render plan", () => {
+    const file = createDiffFile();
+
+    const split = measureDiffSectionMetrics(file, "split", true, theme);
+    const stack = measureDiffSectionMetrics(file, "stack", true, theme);
+
+    expect(split.bodyHeight).toBeGreaterThan(0);
+    expect(stack.bodyHeight).toBeGreaterThan(split.bodyHeight);
+    expect(split.hunkBounds.get(0)?.height).toBeGreaterThan(0);
+    expect(stack.hunkBounds.get(0)?.height).toBeGreaterThan(split.hunkBounds.get(0)?.height ?? 0);
+  });
+
+  test("accounts for visible inline notes without moving the hunk anchor", () => {
+    const file = createDiffFile();
+    const visibleAgentNotes: VisibleAgentNote[] = [
+      {
+        id: "annotation:example:0",
+        annotation: {
+          newRange: [1, 1],
+          rationale: "Keep note height in section metrics.",
+          summary: "Explain the change",
+        },
+      },
+    ];
+
+    const baseMetrics = measureDiffSectionMetrics(file, "split", true, theme, [], 120);
+    const noteMetrics = measureDiffSectionMetrics(
+      file,
+      "split",
+      true,
+      theme,
+      visibleAgentNotes,
+      120,
+    );
+
+    expect(noteMetrics.bodyHeight).toBeGreaterThan(baseMetrics.bodyHeight);
+    expect(noteMetrics.hunkAnchorRows.get(0)).toBe(baseMetrics.hunkAnchorRows.get(0));
+    expect(noteMetrics.rowMetrics.some((row) => row.key.startsWith("inline-note:"))).toBe(true);
+  });
+
+  test("wraps long rows into taller section metrics when wrapping is enabled", () => {
+    const file = createDiffFile({
+      before: lines("const alpha = 1;", "const beta = 2;"),
+      after: lines(
+        "const alpha = 1;",
+        "const beta = 'this is a deliberately long line that should wrap in a narrow viewport';",
+      ),
+      id: "wrapped",
+      path: "wrapped.ts",
+    });
+
+    const nowrapMetrics = measureDiffSectionMetrics(
+      file,
+      "stack",
+      true,
+      theme,
+      [],
+      32,
+      true,
+      false,
+    );
+    const wrappedMetrics = measureDiffSectionMetrics(
+      file,
+      "stack",
+      true,
+      theme,
+      [],
+      32,
+      true,
+      true,
+    );
+
+    expect(wrappedMetrics.bodyHeight).toBeGreaterThan(nowrapMetrics.bodyHeight);
+    expect(wrappedMetrics.hunkBounds.get(0)?.height).toBeGreaterThan(
+      nowrapMetrics.hunkBounds.get(0)?.height ?? 0,
+    );
+  });
+
+  test("returns a one-row placeholder for files with no visible hunks", () => {
+    const file = createDiffFile({
+      after: "const stable = true;\n",
+      before: "const stable = true;\n",
+      id: "empty",
+      path: "empty.ts",
+    });
+
+    const metrics = measureDiffSectionMetrics(file, "split", true, theme);
+
+    expect(file.metadata.hunks).toHaveLength(0);
+    expect(metrics.bodyHeight).toBe(1);
+    expect(metrics.hunkBounds.size).toBe(0);
+    expect(metrics.rowMetrics).toEqual([]);
+  });
+
+  test("can measure a header-only hunk stream without line rows", () => {
+    const file = createHeaderOnlyDiffFile();
+
+    const metrics = measureDiffSectionMetrics(file, "split", true, theme);
+
+    expect(file.metadata.hunks).toHaveLength(1);
+    expect(metrics.bodyHeight).toBe(1);
+    expect(metrics.hunkAnchorRows.size).toBe(1);
+    expect(metrics.hunkAnchorRows.get(0)).toBe(0);
+    expect(metrics.hunkBounds.get(0)).toMatchObject({ height: 1, top: 0 });
+    expect(metrics.rowMetrics).toHaveLength(1);
+    expect(metrics.rowMetrics[0]?.key).toContain(":header:");
+  });
+});