refactor(snapshot): store unified patches in file diffs (#21244)
Co-authored-by: Adam <2363879+adamdotdevin@users.noreply.github.com>
This commit is contained in:
@@ -2124,7 +2124,7 @@ function ApplyPatch(props: ToolProps<typeof ApplyPatchTool>) {
|
||||
</text>
|
||||
}
|
||||
>
|
||||
<Diff diff={file.diff} filePath={file.filePath} />
|
||||
<Diff diff={file.patch} filePath={file.filePath} />
|
||||
<Diagnostics diagnostics={props.metadata.diagnostics} filePath={file.movePath ?? file.filePath} />
|
||||
</Show>
|
||||
</BlockTool>
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { Effect, Layer, ServiceMap, Stream } from "effect"
|
||||
import { formatPatch, structuredPatch } from "diff"
|
||||
import path from "path"
|
||||
import { Bus } from "@/bus"
|
||||
import { BusEvent } from "@/bus/bus-event"
|
||||
@@ -7,7 +8,6 @@ import { makeRuntime } from "@/effect/run-service"
|
||||
import { AppFileSystem } from "@/filesystem"
|
||||
import { FileWatcher } from "@/file/watcher"
|
||||
import { Git } from "@/git"
|
||||
import { Snapshot } from "@/snapshot"
|
||||
import { Log } from "@/util/log"
|
||||
import { Instance } from "./instance"
|
||||
import z from "zod"
|
||||
@@ -49,6 +49,8 @@ export namespace Vcs {
|
||||
map: Map<string, { additions: number; deletions: number }>,
|
||||
) {
|
||||
const base = ref ? yield* git.prefix(cwd) : ""
|
||||
const patch = (file: string, before: string, after: string) =>
|
||||
formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER }))
|
||||
const next = yield* Effect.forEach(
|
||||
list,
|
||||
(item) =>
|
||||
@@ -58,12 +60,11 @@ export namespace Vcs {
|
||||
const stat = map.get(item.file)
|
||||
return {
|
||||
file: item.file,
|
||||
before,
|
||||
after,
|
||||
patch: patch(item.file, before, after),
|
||||
additions: stat?.additions ?? (item.status === "added" ? count(after) : 0),
|
||||
deletions: stat?.deletions ?? (item.status === "deleted" ? count(before) : 0),
|
||||
status: item.status,
|
||||
} satisfies Snapshot.FileDiff
|
||||
} satisfies FileDiff
|
||||
}),
|
||||
{ concurrency: 8 },
|
||||
)
|
||||
@@ -125,11 +126,24 @@ export namespace Vcs {
|
||||
})
|
||||
export type Info = z.infer<typeof Info>
|
||||
|
||||
export const FileDiff = z
|
||||
.object({
|
||||
file: z.string(),
|
||||
patch: z.string(),
|
||||
additions: z.number(),
|
||||
deletions: z.number(),
|
||||
status: z.enum(["added", "deleted", "modified"]).optional(),
|
||||
})
|
||||
.meta({
|
||||
ref: "VcsFileDiff",
|
||||
})
|
||||
export type FileDiff = z.infer<typeof FileDiff>
|
||||
|
||||
export interface Interface {
|
||||
readonly init: () => Effect.Effect<void>
|
||||
readonly branch: () => Effect.Effect<string | undefined>
|
||||
readonly defaultBranch: () => Effect.Effect<string | undefined>
|
||||
readonly diff: (mode: Mode) => Effect.Effect<Snapshot.FileDiff[]>
|
||||
readonly diff: (mode: Mode) => Effect.Effect<FileDiff[]>
|
||||
}
|
||||
|
||||
interface State {
|
||||
|
||||
@@ -154,7 +154,7 @@ export const InstanceRoutes = (upgrade: UpgradeWebSocket, app: Hono = new Hono()
|
||||
description: "VCS diff",
|
||||
content: {
|
||||
"application/json": {
|
||||
schema: resolver(Snapshot.FileDiff.array()),
|
||||
schema: resolver(Vcs.FileDiff.array()),
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -59,7 +59,7 @@ export namespace ShareNext {
|
||||
}
|
||||
| {
|
||||
type: "session_diff"
|
||||
data: SDK.FileDiff[]
|
||||
data: SDK.SnapshotFileDiff[]
|
||||
}
|
||||
| {
|
||||
type: "model"
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { NodeFileSystem, NodePath } from "@effect/platform-node"
|
||||
import { Cause, Duration, Effect, Layer, Schedule, Semaphore, ServiceMap, Stream } from "effect"
|
||||
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"
|
||||
import { formatPatch, structuredPatch } from "diff"
|
||||
import path from "path"
|
||||
import z from "zod"
|
||||
import * as CrossSpawnSpawner from "@/effect/cross-spawn-spawner"
|
||||
@@ -22,14 +22,13 @@ export namespace Snapshot {
|
||||
export const FileDiff = z
|
||||
.object({
|
||||
file: z.string(),
|
||||
before: z.string(),
|
||||
after: z.string(),
|
||||
patch: z.string(),
|
||||
additions: z.number(),
|
||||
deletions: z.number(),
|
||||
status: z.enum(["added", "deleted", "modified"]).optional(),
|
||||
})
|
||||
.meta({
|
||||
ref: "FileDiff",
|
||||
ref: "SnapshotFileDiff",
|
||||
})
|
||||
export type FileDiff = z.infer<typeof FileDiff>
|
||||
|
||||
@@ -521,8 +520,6 @@ export namespace Snapshot {
|
||||
const map = new Map<string, { before: string; after: string }>()
|
||||
const dec = new TextDecoder()
|
||||
let i = 0
|
||||
// Parse the default `git cat-file --batch` stream: one header line,
|
||||
// then exactly `size` bytes of blob content, then a trailing newline.
|
||||
for (const ref of refs) {
|
||||
let end = i
|
||||
while (end < out.length && out[end] !== 10) end += 1
|
||||
@@ -620,8 +617,9 @@ export namespace Snapshot {
|
||||
]
|
||||
})
|
||||
const step = 100
|
||||
const patch = (file: string, before: string, after: string) =>
|
||||
formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER }))
|
||||
|
||||
// Keep batches bounded so a large diff does not buffer every blob at once.
|
||||
for (let i = 0; i < rows.length; i += step) {
|
||||
const run = rows.slice(i, i + step)
|
||||
const text = yield* load(run)
|
||||
@@ -631,8 +629,7 @@ export namespace Snapshot {
|
||||
const [before, after] = row.binary ? ["", ""] : text ? [hit.before, hit.after] : yield* show(row)
|
||||
result.push({
|
||||
file: row.file,
|
||||
before,
|
||||
after,
|
||||
patch: row.binary ? "" : patch(row.file, before, after),
|
||||
additions: row.additions,
|
||||
deletions: row.deletions,
|
||||
status: row.status,
|
||||
|
||||
@@ -164,9 +164,7 @@ export const ApplyPatchTool = Tool.define("apply_patch", {
|
||||
filePath: change.filePath,
|
||||
relativePath: path.relative(Instance.worktree, change.movePath ?? change.filePath).replaceAll("\\", "/"),
|
||||
type: change.type,
|
||||
diff: change.diff,
|
||||
before: change.oldContent,
|
||||
after: change.newContent,
|
||||
patch: change.diff,
|
||||
additions: change.additions,
|
||||
deletions: change.deletions,
|
||||
movePath: change.movePath,
|
||||
|
||||
@@ -123,8 +123,7 @@ export const EditTool = Tool.define("edit", {
|
||||
|
||||
const filediff: Snapshot.FileDiff = {
|
||||
file: filePath,
|
||||
before: contentOld,
|
||||
after: contentNew,
|
||||
patch: diff,
|
||||
additions: 0,
|
||||
deletions: 0,
|
||||
}
|
||||
|
||||
@@ -272,8 +272,8 @@ describe("ShareNext", () => {
|
||||
diff: [
|
||||
{
|
||||
file: "a.ts",
|
||||
before: "one",
|
||||
after: "two",
|
||||
patch:
|
||||
"Index: a.ts\n===================================================================\n--- a.ts\t\n+++ a.ts\t\n@@ -1,1 +1,1 @@\n-one\n\\ No newline at end of file\n+two\n\\ No newline at end of file\n",
|
||||
additions: 1,
|
||||
deletions: 1,
|
||||
status: "modified",
|
||||
@@ -285,8 +285,8 @@ describe("ShareNext", () => {
|
||||
diff: [
|
||||
{
|
||||
file: "b.ts",
|
||||
before: "old",
|
||||
after: "new",
|
||||
patch:
|
||||
"Index: b.ts\n===================================================================\n--- b.ts\t\n+++ b.ts\t\n@@ -1,1 +1,1 @@\n-old\n\\ No newline at end of file\n+new\n\\ No newline at end of file\n",
|
||||
additions: 2,
|
||||
deletions: 0,
|
||||
status: "modified",
|
||||
@@ -304,8 +304,7 @@ describe("ShareNext", () => {
|
||||
type: string
|
||||
data: Array<{
|
||||
file: string
|
||||
before: string
|
||||
after: string
|
||||
patch: string
|
||||
additions: number
|
||||
deletions: number
|
||||
status?: string
|
||||
@@ -318,8 +317,8 @@ describe("ShareNext", () => {
|
||||
expect(body.data[0].data).toEqual([
|
||||
{
|
||||
file: "b.ts",
|
||||
before: "old",
|
||||
after: "new",
|
||||
patch:
|
||||
"Index: b.ts\n===================================================================\n--- b.ts\t\n+++ b.ts\t\n@@ -1,1 +1,1 @@\n-old\n\\ No newline at end of file\n+new\n\\ No newline at end of file\n",
|
||||
additions: 2,
|
||||
deletions: 0,
|
||||
status: "modified",
|
||||
|
||||
@@ -974,8 +974,7 @@ test("diffFull with new file additions", async () => {
|
||||
|
||||
const newFileDiff = diffs[0]
|
||||
expect(newFileDiff.file).toBe("new.txt")
|
||||
expect(newFileDiff.before).toBe("")
|
||||
expect(newFileDiff.after).toBe("new content")
|
||||
expect(newFileDiff.patch).toContain("+new content")
|
||||
expect(newFileDiff.additions).toBe(1)
|
||||
expect(newFileDiff.deletions).toBe(0)
|
||||
},
|
||||
@@ -1020,26 +1019,23 @@ test("diffFull with a large interleaved mixed diff", async () => {
|
||||
for (let i = 0; i < ids.length; i++) {
|
||||
const m = map.get(fwd("mix", `${ids[i]}-mod.txt`))
|
||||
expect(m).toBeDefined()
|
||||
expect(m!.before).toBe(`before-${ids[i]}-é\n🙂\nline`)
|
||||
expect(m!.after).toBe(`after-${ids[i]}-é\n🚀\nline`)
|
||||
expect(m!.patch).toContain(`-before-${ids[i]}-é`)
|
||||
expect(m!.patch).toContain(`+after-${ids[i]}-é`)
|
||||
expect(m!.status).toBe("modified")
|
||||
|
||||
const d = map.get(fwd("mix", `${ids[i]}-del.txt`))
|
||||
expect(d).toBeDefined()
|
||||
expect(d!.before).toBe(`gone-${ids[i]}\n你好`)
|
||||
expect(d!.after).toBe("")
|
||||
expect(d!.patch).toContain(`-gone-${ids[i]}`)
|
||||
expect(d!.status).toBe("deleted")
|
||||
|
||||
const a = map.get(fwd("mix", `${ids[i]}-add.txt`))
|
||||
expect(a).toBeDefined()
|
||||
expect(a!.before).toBe("")
|
||||
expect(a!.after).toBe(`new-${ids[i]}\nこんにちは`)
|
||||
expect(a!.patch).toContain(`+new-${ids[i]}`)
|
||||
expect(a!.status).toBe("added")
|
||||
|
||||
const b = map.get(fwd("mix", `${ids[i]}-bin.bin`))
|
||||
expect(b).toBeDefined()
|
||||
expect(b!.before).toBe("")
|
||||
expect(b!.after).toBe("")
|
||||
expect(b!.patch).toBe("")
|
||||
expect(b!.additions).toBe(0)
|
||||
expect(b!.deletions).toBe(0)
|
||||
expect(b!.status).toBe("modified")
|
||||
@@ -1092,8 +1088,8 @@ test("diffFull with file modifications", async () => {
|
||||
|
||||
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.patch).toContain(`-${tmp.extra.bContent}`)
|
||||
expect(modifiedFileDiff.patch).toContain("+modified content")
|
||||
expect(modifiedFileDiff.additions).toBeGreaterThan(0)
|
||||
expect(modifiedFileDiff.deletions).toBeGreaterThan(0)
|
||||
},
|
||||
@@ -1118,8 +1114,7 @@ test("diffFull with file deletions", async () => {
|
||||
|
||||
const removedFileDiff = diffs[0]
|
||||
expect(removedFileDiff.file).toBe("a.txt")
|
||||
expect(removedFileDiff.before).toBe(tmp.extra.aContent)
|
||||
expect(removedFileDiff.after).toBe("")
|
||||
expect(removedFileDiff.patch).toContain(`-${tmp.extra.aContent}`)
|
||||
expect(removedFileDiff.additions).toBe(0)
|
||||
expect(removedFileDiff.deletions).toBe(1)
|
||||
},
|
||||
@@ -1144,8 +1139,8 @@ test("diffFull with multiple line additions", async () => {
|
||||
|
||||
const multiDiff = diffs[0]
|
||||
expect(multiDiff.file).toBe("multi.txt")
|
||||
expect(multiDiff.before).toBe("")
|
||||
expect(multiDiff.after).toBe("line1\nline2\nline3")
|
||||
expect(multiDiff.patch).toContain("+line1")
|
||||
expect(multiDiff.patch).toContain("+line3")
|
||||
expect(multiDiff.additions).toBe(3)
|
||||
expect(multiDiff.deletions).toBe(0)
|
||||
},
|
||||
@@ -1171,15 +1166,13 @@ test("diffFull with addition and deletion", async () => {
|
||||
|
||||
const addedFileDiff = diffs.find((d) => d.file === "added.txt")
|
||||
expect(addedFileDiff).toBeDefined()
|
||||
expect(addedFileDiff!.before).toBe("")
|
||||
expect(addedFileDiff!.after).toBe("added content")
|
||||
expect(addedFileDiff!.patch).toContain("+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!.before).toBe(tmp.extra.aContent)
|
||||
expect(removedFileDiff!.after).toBe("")
|
||||
expect(removedFileDiff!.patch).toContain(`-${tmp.extra.aContent}`)
|
||||
expect(removedFileDiff!.additions).toBe(0)
|
||||
expect(removedFileDiff!.deletions).toBe(1)
|
||||
},
|
||||
@@ -1263,7 +1256,7 @@ test("diffFull with binary file changes", async () => {
|
||||
|
||||
const binaryDiff = diffs[0]
|
||||
expect(binaryDiff.file).toBe("binary.bin")
|
||||
expect(binaryDiff.before).toBe("")
|
||||
expect(binaryDiff.patch).toBe("")
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
@@ -27,9 +27,7 @@ type AskInput = {
|
||||
filePath: string
|
||||
relativePath: string
|
||||
type: "add" | "update" | "delete" | "move"
|
||||
diff: string
|
||||
before: string
|
||||
after: string
|
||||
patch: string
|
||||
additions: number
|
||||
deletions: number
|
||||
movePath?: string
|
||||
@@ -112,12 +110,12 @@ describe("tool.apply_patch freeform", () => {
|
||||
const addFile = permissionCall.metadata.files.find((f) => f.type === "add")
|
||||
expect(addFile).toBeDefined()
|
||||
expect(addFile!.relativePath).toBe("nested/new.txt")
|
||||
expect(addFile!.after).toBe("created\n")
|
||||
expect(addFile!.patch).toContain("+created")
|
||||
|
||||
const updateFile = permissionCall.metadata.files.find((f) => f.type === "update")
|
||||
expect(updateFile).toBeDefined()
|
||||
expect(updateFile!.before).toContain("line2")
|
||||
expect(updateFile!.after).toContain("changed")
|
||||
expect(updateFile!.patch).toContain("-line2")
|
||||
expect(updateFile!.patch).toContain("+changed")
|
||||
|
||||
const added = await fs.readFile(path.join(fixture.path, "nested", "new.txt"), "utf-8")
|
||||
expect(added).toBe("created\n")
|
||||
@@ -151,8 +149,8 @@ describe("tool.apply_patch freeform", () => {
|
||||
expect(moveFile.type).toBe("move")
|
||||
expect(moveFile.relativePath).toBe("renamed/dir/name.txt")
|
||||
expect(moveFile.movePath).toBe(path.join(fixture.path, "renamed/dir/name.txt"))
|
||||
expect(moveFile.before).toBe("old content\n")
|
||||
expect(moveFile.after).toBe("new content\n")
|
||||
expect(moveFile.patch).toContain("-old content")
|
||||
expect(moveFile.patch).toContain("+new content")
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user