Skip to content

Commit 98d33f5

Browse files
committed
test: patch seekSequence matching and Command.hints placeholder extraction
Cover two previously untested code paths: (1) Patch.deriveNewContentsFromChunks with seekSequence's 4-pass matching (exact, rstrip, trim, Unicode-normalized) and EOF anchoring/change_context seeking; (2) Command.hints() argument placeholder extraction that drives TUI argument prompts for slash commands. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> https://claude.ai/code/session_01Nvdry9ChLovXGjHNS8kR7R
1 parent 544903f commit 98d33f5

2 files changed

Lines changed: 313 additions & 0 deletions

File tree

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { describe, test, expect } from "bun:test"
2+
import { Command } from "../../src/command"
3+
4+
/**
5+
* Tests for Command.hints() — the pure function that extracts argument
6+
* placeholders ($1, $2, ..., $ARGUMENTS) from command templates.
7+
*
8+
* These hints drive the TUI's argument prompt display. If hints are wrong,
9+
* users see incorrect or missing argument suggestions when invoking commands
10+
* like /review, /init, or custom commands.
11+
*/
12+
13+
describe("Command.hints", () => {
14+
test("returns empty array for template with no placeholders", () => {
15+
expect(Command.hints("Run all tests")).toEqual([])
16+
})
17+
18+
test("extracts single numbered placeholder", () => {
19+
expect(Command.hints("Review commit $1")).toEqual(["$1"])
20+
})
21+
22+
test("extracts multiple numbered placeholders in sorted order", () => {
23+
expect(Command.hints("Compare $2 against $1")).toEqual(["$1", "$2"])
24+
})
25+
26+
test("deduplicates repeated placeholders", () => {
27+
expect(Command.hints("Use $1 then reuse $1 again")).toEqual(["$1"])
28+
})
29+
30+
test("extracts $ARGUMENTS placeholder", () => {
31+
expect(Command.hints("Execute with $ARGUMENTS")).toEqual(["$ARGUMENTS"])
32+
})
33+
34+
test("extracts both numbered and $ARGUMENTS, numbered first", () => {
35+
expect(Command.hints("Run $1 with $ARGUMENTS")).toEqual(["$1", "$ARGUMENTS"])
36+
})
37+
38+
test("handles multi-digit placeholders like $10 (lexicographic sort)", () => {
39+
const result = Command.hints("Lots of args: $1 $2 $10")
40+
// JavaScript's default sort is lexicographic: "$10" < "$2" because "1" < "2"
41+
expect(result).toEqual(["$1", "$10", "$2"])
42+
})
43+
44+
test("returns empty for empty template string", () => {
45+
expect(Command.hints("")).toEqual([])
46+
})
47+
48+
test("does not match $ followed by letters (not ARGUMENTS)", () => {
49+
expect(Command.hints("Use $FOO and $BAR")).toEqual([])
50+
})
51+
52+
test("$ARGUMENTS is case-sensitive", () => {
53+
expect(Command.hints("Use $arguments")).toEqual([])
54+
})
55+
56+
test("handles template with only whitespace", () => {
57+
expect(Command.hints(" \n\t ")).toEqual([])
58+
})
59+
60+
test("handles $0 as a valid numbered placeholder", () => {
61+
expect(Command.hints("$0 is first")).toEqual(["$0"])
62+
})
63+
})
Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
1+
import { describe, test, expect, beforeEach, afterEach } from "bun:test"
2+
import { Patch } from "../../src/patch"
3+
import * as fs from "fs/promises"
4+
import * as path from "path"
5+
import { tmpdir } from "os"
6+
7+
/**
8+
* Tests for Patch.deriveNewContentsFromChunks — the core function that applies
9+
* update chunks to file content using seekSequence's multi-pass matching.
10+
*
11+
* seekSequence tries 4 comparison strategies in order:
12+
* 1. Exact match
13+
* 2. Trailing whitespace trimmed (rstrip)
14+
* 3. Both-end whitespace trimmed (trim)
15+
* 4. Unicode-normalized + trimmed
16+
*
17+
* These tests verify that real-world patch application succeeds even when the
18+
* LLM-generated patch text has minor whitespace or Unicode differences from
19+
* the actual file content — a common source of "Failed to find expected lines"
20+
* errors for users.
21+
*/
22+
23+
describe("Patch.deriveNewContentsFromChunks — seekSequence matching", () => {
24+
let tempDir: string
25+
26+
beforeEach(async () => {
27+
tempDir = await fs.mkdtemp(path.join(tmpdir(), "seek-test-"))
28+
})
29+
30+
afterEach(async () => {
31+
await fs.rm(tempDir, { recursive: true, force: true })
32+
})
33+
34+
test("exact match: replaces old_lines with new_lines", () => {
35+
const filePath = path.join(tempDir, "exact.txt")
36+
const content = "line1\nline2\nline3\n"
37+
require("fs").writeFileSync(filePath, content)
38+
39+
const result = Patch.deriveNewContentsFromChunks(filePath, [
40+
{
41+
old_lines: ["line2"],
42+
new_lines: ["REPLACED"],
43+
},
44+
])
45+
46+
expect(result.content).toBe("line1\nREPLACED\nline3\n")
47+
expect(result.unified_diff).toContain("-line2")
48+
expect(result.unified_diff).toContain("+REPLACED")
49+
})
50+
51+
test("rstrip pass: matches despite trailing whitespace differences", () => {
52+
const filePath = path.join(tempDir, "rstrip.txt")
53+
// File has trailing spaces on line2
54+
const content = "line1\nline2 \nline3\n"
55+
require("fs").writeFileSync(filePath, content)
56+
57+
// Patch references line2 without trailing spaces
58+
const result = Patch.deriveNewContentsFromChunks(filePath, [
59+
{
60+
old_lines: ["line2"],
61+
new_lines: ["REPLACED"],
62+
},
63+
])
64+
65+
expect(result.content).toContain("REPLACED")
66+
})
67+
68+
test("trim pass: matches despite leading whitespace differences", () => {
69+
const filePath = path.join(tempDir, "trim.txt")
70+
// File has extra leading spaces
71+
const content = " function foo() {\n return 1\n }\n"
72+
require("fs").writeFileSync(filePath, content)
73+
74+
// Patch references without leading spaces
75+
const result = Patch.deriveNewContentsFromChunks(filePath, [
76+
{
77+
old_lines: ["return 1"],
78+
new_lines: ["return 42"],
79+
},
80+
])
81+
82+
expect(result.content).toContain("return 42")
83+
})
84+
85+
test("unicode pass: matches smart quotes to ASCII quotes", () => {
86+
const filePath = path.join(tempDir, "unicode.txt")
87+
// File uses Unicode smart quotes (common in copy-pasted code)
88+
const content = 'const msg = \u201CHello World\u201D\n'
89+
require("fs").writeFileSync(filePath, content)
90+
91+
// Patch uses ASCII double quotes
92+
const result = Patch.deriveNewContentsFromChunks(filePath, [
93+
{
94+
old_lines: ['const msg = "Hello World"'],
95+
new_lines: ['const msg = "Goodbye World"'],
96+
},
97+
])
98+
99+
expect(result.content).toContain("Goodbye World")
100+
})
101+
102+
test("unicode pass: matches em-dash to hyphen", () => {
103+
const filePath = path.join(tempDir, "emdash.txt")
104+
// File uses Unicode em-dash
105+
const content = "value \u2014 description\n"
106+
require("fs").writeFileSync(filePath, content)
107+
108+
// Patch uses ASCII hyphen
109+
const result = Patch.deriveNewContentsFromChunks(filePath, [
110+
{
111+
old_lines: ["value - description"],
112+
new_lines: ["value - updated"],
113+
},
114+
])
115+
116+
expect(result.content).toContain("updated")
117+
})
118+
119+
test("is_end_of_file: anchors match to end of file", () => {
120+
const filePath = path.join(tempDir, "eof.txt")
121+
const content = "line1\nline2\nline3\nline2\n"
122+
require("fs").writeFileSync(filePath, content)
123+
124+
// Should match the LAST occurrence of "line2" because is_end_of_file is true
125+
const result = Patch.deriveNewContentsFromChunks(filePath, [
126+
{
127+
old_lines: ["line2"],
128+
new_lines: ["LAST"],
129+
is_end_of_file: true,
130+
},
131+
])
132+
133+
// Exact expected output: first "line2" untouched, last "line2" replaced
134+
expect(result.content).toBe("line1\nline2\nline3\nLAST\n")
135+
})
136+
137+
test("change_context: seeks to context line before matching old_lines", () => {
138+
const filePath = path.join(tempDir, "context.txt")
139+
const content = "function foo() {\n return 1\n}\nfunction bar() {\n return 1\n}\n"
140+
require("fs").writeFileSync(filePath, content)
141+
142+
// Use change_context to target the return inside bar(), not foo()
143+
const result = Patch.deriveNewContentsFromChunks(filePath, [
144+
{
145+
old_lines: [" return 1"],
146+
new_lines: [" return 99"],
147+
change_context: "function bar() {",
148+
},
149+
])
150+
151+
expect(result.content).toContain("function foo() {\n return 1")
152+
expect(result.content).toContain("function bar() {\n return 99")
153+
})
154+
155+
test("throws when old_lines cannot be found", () => {
156+
const filePath = path.join(tempDir, "missing.txt")
157+
require("fs").writeFileSync(filePath, "hello\nworld\n")
158+
159+
expect(() =>
160+
Patch.deriveNewContentsFromChunks(filePath, [
161+
{
162+
old_lines: ["nonexistent line"],
163+
new_lines: ["replacement"],
164+
},
165+
]),
166+
).toThrow("Failed to find expected lines")
167+
})
168+
169+
test("throws when file does not exist", () => {
170+
expect(() =>
171+
Patch.deriveNewContentsFromChunks("/tmp/nonexistent-file-12345.txt", [
172+
{
173+
old_lines: ["x"],
174+
new_lines: ["y"],
175+
},
176+
]),
177+
).toThrow("Failed to read file")
178+
})
179+
180+
test("multiple chunks applied in sequence", () => {
181+
const filePath = path.join(tempDir, "multi.txt")
182+
const content = "alpha\nbeta\ngamma\ndelta\n"
183+
require("fs").writeFileSync(filePath, content)
184+
185+
const result = Patch.deriveNewContentsFromChunks(filePath, [
186+
{
187+
old_lines: ["beta"],
188+
new_lines: ["BETA"],
189+
},
190+
{
191+
old_lines: ["delta"],
192+
new_lines: ["DELTA"],
193+
},
194+
])
195+
196+
expect(result.content).toBe("alpha\nBETA\ngamma\nDELTA\n")
197+
})
198+
199+
test("pure addition chunk (empty old_lines) appends content", () => {
200+
const filePath = path.join(tempDir, "append.txt")
201+
require("fs").writeFileSync(filePath, "existing\n")
202+
203+
const result = Patch.deriveNewContentsFromChunks(filePath, [
204+
{
205+
old_lines: [],
206+
new_lines: ["new_line"],
207+
},
208+
])
209+
210+
expect(result.content).toContain("existing")
211+
expect(result.content).toContain("new_line")
212+
})
213+
})
214+
215+
describe("Patch.parsePatch — stripHeredoc handling", () => {
216+
test("parses patch wrapped in heredoc with cat <<'EOF'", () => {
217+
const input = `cat <<'EOF'
218+
*** Begin Patch
219+
*** Add File: hello.txt
220+
+Hello
221+
*** End Patch
222+
EOF`
223+
224+
const result = Patch.parsePatch(input)
225+
expect(result.hunks).toHaveLength(1)
226+
expect(result.hunks[0].type).toBe("add")
227+
})
228+
229+
test("parses patch wrapped in heredoc with <<HEREDOC", () => {
230+
const input = `<<HEREDOC
231+
*** Begin Patch
232+
*** Delete File: old.txt
233+
*** End Patch
234+
HEREDOC`
235+
236+
const result = Patch.parsePatch(input)
237+
expect(result.hunks).toHaveLength(1)
238+
expect(result.hunks[0].type).toBe("delete")
239+
})
240+
241+
test("parses unwrapped patch text unchanged", () => {
242+
const input = `*** Begin Patch
243+
*** Add File: test.txt
244+
+content
245+
*** End Patch`
246+
247+
const result = Patch.parsePatch(input)
248+
expect(result.hunks).toHaveLength(1)
249+
})
250+
})

0 commit comments

Comments
 (0)