core: provide line-level statistics in file diffs to help users understand the scale of changes

2026-01-07 18:04:54 +01:00 · 2025-10-21 11:54:18 -04:00
parent a84826061d
commit e9996342a7
3 changed files with 246 additions and 115 deletions
--- a/packages/opencode/src/session/index.ts
+++ b/packages/opencode/src/session/index.ts
@@ -147,12 +147,7 @@ export namespace Session {
    })
  })

-  export async function createNext(input: {
-    id?: string
-    title?: string
-    parentID?: string
-    directory: string
-  }) {
+  export async function createNext(input: { id?: string; title?: string; parentID?: string; directory: string }) {
    const result: Info = {
      id: Identifier.descending("session", input.id),
      version: Installation.VERSION,
@@ -372,9 +367,7 @@ export namespace Session {
          .add(new Decimal(tokens.input).mul(input.model.cost?.input ?? 0).div(1_000_000))
          .add(new Decimal(tokens.output).mul(input.model.cost?.output ?? 0).div(1_000_000))
          .add(new Decimal(tokens.cache.read).mul(input.model.cost?.cache_read ?? 0).div(1_000_000))
-          .add(
-            new Decimal(tokens.cache.write).mul(input.model.cost?.cache_write ?? 0).div(1_000_000),
-          )
+          .add(new Decimal(tokens.cache.write).mul(input.model.cost?.cache_write ?? 0).div(1_000_000))
          .toNumber(),
        tokens,
      }
--- a/packages/opencode/src/snapshot/index.ts
+++ b/packages/opencode/src/snapshot/index.ts
@@ -26,15 +26,8 @@ export namespace Snapshot {
        .nothrow()
      log.info("initialized")
    }
-    await $`git --git-dir ${git} add .`
-      .quiet()
-      .cwd(Instance.directory)
-      .nothrow()
-    const hash = await $`git --git-dir ${git} write-tree`
-      .quiet()
-      .cwd(Instance.directory)
-      .nothrow()
-      .text()
+    await $`git --git-dir ${git} add .`.quiet().cwd(Instance.directory).nothrow()
+    const hash = await $`git --git-dir ${git} write-tree`.quiet().cwd(Instance.directory).nothrow().text()
    log.info("tracking", { hash, cwd: Instance.directory, git })
    return hash.trim()
  }
@@ -47,14 +40,8 @@ export namespace Snapshot {

  export async function patch(hash: string): Promise<Patch> {
    const git = gitdir()
-    await $`git --git-dir ${git} add .`
-      .quiet()
-      .cwd(Instance.directory)
-      .nothrow()
-    const result = await $`git --git-dir ${git} diff --name-only ${hash} -- .`
-      .quiet()
-      .cwd(Instance.directory)
-      .nothrow()
+    await $`git --git-dir ${git} add .`.quiet().cwd(Instance.directory).nothrow()
+    const result = await $`git --git-dir ${git} diff --name-only ${hash} -- .`.quiet().cwd(Instance.directory).nothrow()

    // If git diff fails, return empty patch
    if (result.exitCode !== 0) {
@@ -77,11 +64,10 @@ export namespace Snapshot {
  export async function restore(snapshot: string) {
    log.info("restore", { commit: snapshot })
    const git = gitdir()
-    const result =
-      await $`git --git-dir=${git} read-tree ${snapshot} && git --git-dir=${git} checkout-index -a -f`
-        .quiet()
-        .cwd(Instance.worktree)
-        .nothrow()
+    const result = await $`git --git-dir=${git} read-tree ${snapshot} && git --git-dir=${git} checkout-index -a -f`
+      .quiet()
+      .cwd(Instance.worktree)
+      .nothrow()

    if (result.exitCode !== 0) {
      log.error("failed to restore snapshot", {
@@ -100,18 +86,16 @@ export namespace Snapshot {
      for (const file of item.files) {
        if (files.has(file)) continue
        log.info("reverting", { file, hash: item.hash })
-        const result =
-          await $`git --git-dir=${git} checkout ${item.hash} -- ${file}`
+        const result = await $`git --git-dir=${git} checkout ${item.hash} -- ${file}`
+          .quiet()
+          .cwd(Instance.worktree)
+          .nothrow()
+        if (result.exitCode !== 0) {
+          const relativePath = path.relative(Instance.worktree, file)
+          const checkTree = await $`git --git-dir=${git} ls-tree ${item.hash} -- ${relativePath}`
            .quiet()
            .cwd(Instance.worktree)
            .nothrow()
-        if (result.exitCode !== 0) {
-          const relativePath = path.relative(Instance.worktree, file)
-          const checkTree =
-            await $`git --git-dir=${git} ls-tree ${item.hash} -- ${relativePath}`
-              .quiet()
-              .cwd(Instance.worktree)
-              .nothrow()
          if (checkTree.exitCode === 0 && checkTree.text().trim()) {
            log.info("file existed in snapshot but checkout failed, keeping", {
              file,
@@ -128,14 +112,8 @@ export namespace Snapshot {

  export async function diff(hash: string) {
    const git = gitdir()
-    await $`git --git-dir ${git} add .`
-      .quiet()
-      .cwd(Instance.directory)
-      .nothrow()
-    const result = await $`git --git-dir=${git} diff ${hash} -- .`
-      .quiet()
-      .cwd(Instance.worktree)
-      .nothrow()
+    await $`git --git-dir ${git} add .`.quiet().cwd(Instance.directory).nothrow()
+    const result = await $`git --git-dir=${git} diff ${hash} -- .`.quiet().cwd(Instance.worktree).nothrow()

    if (result.exitCode !== 0) {
      log.warn("failed to get diff", {
@@ -153,37 +131,33 @@ export namespace Snapshot {
  export const FileDiff = z
    .object({
      file: z.string(),
-      left: z.string(),
-      right: z.string(),
+      before: z.string(),
+      after: z.string(),
+      additions: z.number(),
+      deletions: z.number(),
    })
    .meta({
      ref: "FileDiff",
    })
  export type FileDiff = z.infer<typeof FileDiff>
-  export async function diffFull(
-    from: string,
-    to: string,
-  ): Promise<FileDiff[]> {
+  export async function diffFull(from: string, to: string): Promise<FileDiff[]> {
    const git = gitdir()
    const result: FileDiff[] = []
-    for await (const line of $`git --git-dir=${git} diff --name-only ${from} ${to} -- .`
+    for await (const line of $`git --git-dir=${git} diff --numstat ${from} ${to} -- .`
      .quiet()
      .cwd(Instance.directory)
      .nothrow()
      .lines()) {
      if (!line) continue
-      const left = await $`git --git-dir=${git} show ${from}:${line}`
-        .quiet()
-        .nothrow()
-        .text()
-      const right = await $`git --git-dir=${git} show ${to}:${line}`
-        .quiet()
-        .nothrow()
-        .text()
+      const [additions, deletions, file] = line.split("\t")
+      const before = await $`git --git-dir=${git} show ${from}:${file}`.quiet().nothrow().text()
+      const after = await $`git --git-dir=${git} show ${to}:${file}`.quiet().nothrow().text()
      result.push({
-        file: line,
-        left,
-        right,
+        file,
+        before,
+        after,
+        additions: parseInt(additions),
+        deletions: parseInt(deletions),
      })
    }
    return result
--- a/packages/opencode/test/snapshot/snapshot.test.ts
+++ b/packages/opencode/test/snapshot/snapshot.test.ts
@@ -33,9 +33,7 @@ test("tracks deleted files correctly", async () => {

      await $`rm ${tmp.path}/a.txt`.quiet()

-      expect((await Snapshot.patch(before!)).files).toContain(
-        `${tmp.path}/a.txt`,
-      )
+      expect((await Snapshot.patch(before!)).files).toContain(`${tmp.path}/a.txt`)
    },
  })
 })
@@ -93,15 +91,11 @@ test("multiple file operations", async () => {

      await Snapshot.revert([await Snapshot.patch(before!)])

-      expect(await Bun.file(`${tmp.path}/a.txt`).text()).toBe(
-        tmp.extra.aContent,
-      )
+      expect(await Bun.file(`${tmp.path}/a.txt`).text()).toBe(tmp.extra.aContent)
      expect(await Bun.file(`${tmp.path}/c.txt`).exists()).toBe(false)
      // Note: revert currently only removes files, not directories
      // The empty directory will remain
-      expect(await Bun.file(`${tmp.path}/b.txt`).text()).toBe(
-        tmp.extra.bContent,
-      )
+      expect(await Bun.file(`${tmp.path}/b.txt`).text()).toBe(tmp.extra.bContent)
    },
  })
 })
@@ -129,10 +123,7 @@ test("binary file handling", async () => {
      const before = await Snapshot.track()
      expect(before).toBeTruthy()

-      await Bun.write(
-        `${tmp.path}/image.png`,
-        new Uint8Array([0x89, 0x50, 0x4e, 0x47]),
-      )
+      await Bun.write(`${tmp.path}/image.png`, new Uint8Array([0x89, 0x50, 0x4e, 0x47]))

      const patch = await Snapshot.patch(before!)
      expect(patch.files).toContain(`${tmp.path}/image.png`)
@@ -153,9 +144,7 @@ test("symlink handling", async () => {

      await $`ln -s ${tmp.path}/a.txt ${tmp.path}/link.txt`.quiet()

-      expect((await Snapshot.patch(before!)).files).toContain(
-        `${tmp.path}/link.txt`,
-      )
+      expect((await Snapshot.patch(before!)).files).toContain(`${tmp.path}/link.txt`)
    },
  })
 })
@@ -170,9 +159,7 @@ test("large file handling", async () => {

      await Bun.write(`${tmp.path}/large.txt`, "x".repeat(1024 * 1024))

-      expect((await Snapshot.patch(before!)).files).toContain(
-        `${tmp.path}/large.txt`,
-      )
+      expect((await Snapshot.patch(before!)).files).toContain(`${tmp.path}/large.txt`)
    },
  })
 })
@@ -190,9 +177,7 @@ test("nested directory revert", async () => {

      await Snapshot.revert([await Snapshot.patch(before!)])

-      expect(
-        await Bun.file(`${tmp.path}/level1/level2/level3/deep.txt`).exists(),
-      ).toBe(false)
+      expect(await Bun.file(`${tmp.path}/level1/level2/level3/deep.txt`).exists()).toBe(false)
    },
  })
 })
@@ -226,9 +211,7 @@ test("revert with empty patches", async () => {
      expect(Snapshot.revert([])).resolves.toBeUndefined()

      // Should not crash with patches that have empty file lists
-      expect(
-        Snapshot.revert([{ hash: "dummy", files: [] }]),
-      ).resolves.toBeUndefined()
+      expect(Snapshot.revert([{ hash: "dummy", files: [] }])).resolves.toBeUndefined()
    },
  })
 })
@@ -543,13 +526,9 @@ test("restore function", async () => {
      await Snapshot.restore(before!)

      expect(await Bun.file(`${tmp.path}/a.txt`).exists()).toBe(true)
-      expect(await Bun.file(`${tmp.path}/a.txt`).text()).toBe(
-        tmp.extra.aContent,
-      )
+      expect(await Bun.file(`${tmp.path}/a.txt`).text()).toBe(tmp.extra.aContent)
      expect(await Bun.file(`${tmp.path}/new.txt`).exists()).toBe(true) // New files should remain
-      expect(await Bun.file(`${tmp.path}/b.txt`).text()).toBe(
-        tmp.extra.bContent,
-      )
+      expect(await Bun.file(`${tmp.path}/b.txt`).text()).toBe(tmp.extra.bContent)
    },
  })
 })
@@ -601,14 +580,12 @@ test("revert preserves file that existed in snapshot when deleted then recreated

      expect(await Bun.file(`${tmp.path}/newfile.txt`).exists()).toBe(false)
      expect(await Bun.file(`${tmp.path}/existing.txt`).exists()).toBe(true)
-      expect(await Bun.file(`${tmp.path}/existing.txt`).text()).toBe(
-        "original content",
-      )
+      expect(await Bun.file(`${tmp.path}/existing.txt`).text()).toBe("original content")
    },
  })
 })

-test("diffFull function", async () => {
+test("diffFull with new file additions", async () => {
  await using tmp = await bootstrap()
  await Instance.provide({
    directory: tmp.path,
@@ -617,26 +594,103 @@ test("diffFull function", async () => {
      expect(before).toBeTruthy()

      await Bun.write(`${tmp.path}/new.txt`, "new content")
+
+      const after = await Snapshot.track()
+      expect(after).toBeTruthy()
+
+      const diffs = await Snapshot.diffFull(before!, after!)
+      expect(diffs.length).toBe(1)
+
+      const newFileDiff = diffs[0]
+      expect(newFileDiff.file).toBe("new.txt")
+      expect(newFileDiff.before).toBe("")
+      expect(newFileDiff.after).toBe("new content")
+      expect(newFileDiff.additions).toBe(1)
+      expect(newFileDiff.deletions).toBe(0)
+    },
+  })
+})
+
+test("diffFull with file modifications", async () => {
+  await using tmp = await bootstrap()
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const before = await Snapshot.track()
+      expect(before).toBeTruthy()
+
      await Bun.write(`${tmp.path}/b.txt`, "modified content")

      const after = await Snapshot.track()
      expect(after).toBeTruthy()

      const diffs = await Snapshot.diffFull(before!, after!)
-      expect(diffs.length).toBe(2)
+      expect(diffs.length).toBe(1)

-      const newFileDiff = diffs.find((d) => d.file === "new.txt")
-      expect(newFileDiff).toBeDefined()
-      expect(newFileDiff!.left).toBe("")
-      expect(newFileDiff!.right).toBe("new content")
-
-      const modifiedFileDiff = diffs.find((d) => d.file === "b.txt")
-      expect(modifiedFileDiff).toBeDefined()
-      expect(modifiedFileDiff!.left).toBe(tmp.extra.bContent)
-      expect(modifiedFileDiff!.right).toBe("modified content")
+      const modifiedFileDiff = diffs[0]
+      expect(modifiedFileDiff.file).toBe("b.txt")
+      expect(modifiedFileDiff.before).toBe(tmp.extra.bContent)
+      expect(modifiedFileDiff.after).toBe("modified content")
+      expect(modifiedFileDiff.additions).toBeGreaterThan(0)
+      expect(modifiedFileDiff.deletions).toBeGreaterThan(0)
    },
  })
+})

+test("diffFull with file deletions", async () => {
+  await using tmp = await bootstrap()
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const before = await Snapshot.track()
+      expect(before).toBeTruthy()
+
+      await $`rm ${tmp.path}/a.txt`.quiet()
+
+      const after = await Snapshot.track()
+      expect(after).toBeTruthy()
+
+      const diffs = await Snapshot.diffFull(before!, after!)
+      expect(diffs.length).toBe(1)
+
+      const removedFileDiff = diffs[0]
+      expect(removedFileDiff.file).toBe("a.txt")
+      expect(removedFileDiff.before).toBe(tmp.extra.aContent)
+      expect(removedFileDiff.after).toBe("")
+      expect(removedFileDiff.additions).toBe(0)
+      expect(removedFileDiff.deletions).toBe(1)
+    },
+  })
+})
+
+test("diffFull with multiple line additions", async () => {
+  await using tmp = await bootstrap()
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const before = await Snapshot.track()
+      expect(before).toBeTruthy()
+
+      await Bun.write(`${tmp.path}/multi.txt`, "line1\nline2\nline3")
+
+      const after = await Snapshot.track()
+      expect(after).toBeTruthy()
+
+      const diffs = await Snapshot.diffFull(before!, after!)
+      expect(diffs.length).toBe(1)
+
+      const multiDiff = diffs[0]
+      expect(multiDiff.file).toBe("multi.txt")
+      expect(multiDiff.before).toBe("")
+      expect(multiDiff.after).toBe("line1\nline2\nline3")
+      expect(multiDiff.additions).toBe(3)
+      expect(multiDiff.deletions).toBe(0)
+    },
+  })
+})
+
+test("diffFull with addition and deletion", async () => {
+  await using tmp = await bootstrap()
  await Instance.provide({
    directory: tmp.path,
    fn: async () => {
@@ -654,13 +708,123 @@ test("diffFull function", async () => {

      const addedFileDiff = diffs.find((d) => d.file === "added.txt")
      expect(addedFileDiff).toBeDefined()
-      expect(addedFileDiff!.left).toBe("")
-      expect(addedFileDiff!.right).toBe("added content")
+      expect(addedFileDiff!.before).toBe("")
+      expect(addedFileDiff!.after).toBe("added content")
+      expect(addedFileDiff!.additions).toBe(1)
+      expect(addedFileDiff!.deletions).toBe(0)

      const removedFileDiff = diffs.find((d) => d.file === "a.txt")
      expect(removedFileDiff).toBeDefined()
-      expect(removedFileDiff!.left).toBe(tmp.extra.aContent)
-      expect(removedFileDiff!.right).toBe("")
+      expect(removedFileDiff!.before).toBe(tmp.extra.aContent)
+      expect(removedFileDiff!.after).toBe("")
+      expect(removedFileDiff!.additions).toBe(0)
+      expect(removedFileDiff!.deletions).toBe(1)
+    },
+  })
+})
+
+test("diffFull with multiple additions and deletions", async () => {
+  await using tmp = await bootstrap()
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const before = await Snapshot.track()
+      expect(before).toBeTruthy()
+
+      await Bun.write(`${tmp.path}/multi1.txt`, "line1\nline2\nline3")
+      await Bun.write(`${tmp.path}/multi2.txt`, "single line")
+      await $`rm ${tmp.path}/a.txt`.quiet()
+      await $`rm ${tmp.path}/b.txt`.quiet()
+
+      const after = await Snapshot.track()
+      expect(after).toBeTruthy()
+
+      const diffs = await Snapshot.diffFull(before!, after!)
+      expect(diffs.length).toBe(4)
+
+      const multi1Diff = diffs.find((d) => d.file === "multi1.txt")
+      expect(multi1Diff).toBeDefined()
+      expect(multi1Diff!.additions).toBe(3)
+      expect(multi1Diff!.deletions).toBe(0)
+
+      const multi2Diff = diffs.find((d) => d.file === "multi2.txt")
+      expect(multi2Diff).toBeDefined()
+      expect(multi2Diff!.additions).toBe(1)
+      expect(multi2Diff!.deletions).toBe(0)
+
+      const removedADiff = diffs.find((d) => d.file === "a.txt")
+      expect(removedADiff).toBeDefined()
+      expect(removedADiff!.additions).toBe(0)
+      expect(removedADiff!.deletions).toBe(1)
+
+      const removedBDiff = diffs.find((d) => d.file === "b.txt")
+      expect(removedBDiff).toBeDefined()
+      expect(removedBDiff!.additions).toBe(0)
+      expect(removedBDiff!.deletions).toBe(1)
+    },
+  })
+})
+
+test("diffFull with no changes", async () => {
+  await using tmp = await bootstrap()
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const before = await Snapshot.track()
+      expect(before).toBeTruthy()
+
+      const after = await Snapshot.track()
+      expect(after).toBeTruthy()
+
+      const diffs = await Snapshot.diffFull(before!, after!)
+      expect(diffs.length).toBe(0)
+    },
+  })
+})
+
+test("diffFull with binary file changes", async () => {
+  await using tmp = await bootstrap()
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const before = await Snapshot.track()
+      expect(before).toBeTruthy()
+
+      await Bun.write(`${tmp.path}/binary.bin`, new Uint8Array([0x00, 0x01, 0x02, 0x03]))
+
+      const after = await Snapshot.track()
+      expect(after).toBeTruthy()
+
+      const diffs = await Snapshot.diffFull(before!, after!)
+      expect(diffs.length).toBe(1)
+
+      const binaryDiff = diffs[0]
+      expect(binaryDiff.file).toBe("binary.bin")
+      expect(binaryDiff.before).toBe("")
+    },
+  })
+})
+
+test("diffFull with whitespace changes", async () => {
+  await using tmp = await bootstrap()
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      await Bun.write(`${tmp.path}/whitespace.txt`, "line1\nline2")
+      const before = await Snapshot.track()
+      expect(before).toBeTruthy()
+
+      await Bun.write(`${tmp.path}/whitespace.txt`, "line1\n\nline2\n")
+
+      const after = await Snapshot.track()
+      expect(after).toBeTruthy()
+
+      const diffs = await Snapshot.diffFull(before!, after!)
+      expect(diffs.length).toBe(1)
+
+      const whitespaceDiff = diffs[0]
+      expect(whitespaceDiff.file).toBe("whitespace.txt")
+      expect(whitespaceDiff.additions).toBeGreaterThan(0)
    },
  })
 })