Skip to content

Commit e863873

Browse files
Let a patch say which of several identical places it means
A patch names the scope a change is inside in one of two ways: a heading on the "@@" line, or a hunk of nothing but context immediately before the change. Both were discarded -- the heading as envelope, the context block as a hunk that changes nothing -- so a hunk whose context was a closing brace and its indent was searched for across the whole file and matched once per function. Ladder rung 9 sent the same six-line patch six times in a row, told each time to add more context, with the context it needed sitting in the hunk above it. Both forms now travel as a hunk's Anchor. It is never applied and never required to match: its only effect is to narrow where the hunk is looked for, so an anchor naming something the file no longer has leaves the hunk exactly as findable as it was without one. A context block after the last change still anchors nothing and is still dropped, which is the case that forgiveness was added for. Separately, an unprefixed line is accepted as context written without its leading space, which is right inside a hunk and wrong after "*** End Patch": a run that finished its patch and wrote a stray "EOF" had that EOF folded into the last hunk, which then asked the file to contain a line no file contains. Rung 9 passes in 502 seconds after six failing passes. Change-Log: CL-20260803-149 Dev-Log: DL-20260803-159 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 373fc05 commit e863873

3 files changed

Lines changed: 356 additions & 10 deletions

File tree

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
package executor
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
// TestTrailingTextAfterTheEndMarkerIsNotContext is the parse defect that made
9+
// most patches unmatchable.
10+
//
11+
// An unprefixed line inside a hunk is accepted as context written without its
12+
// leading space, which is common and worth accepting. After "*** End Patch" it
13+
// is not context at all: the patch has said it is over. A run that finished its
14+
// patch and then wrote a stray "EOF" — the shape a shell heredoc leaves behind,
15+
// and one models reproduce — had that EOF folded into the last hunk, so the
16+
// hunk was searched for as the file's own text plus a line no file contains.
17+
//
18+
// Proven to discriminate: against the previous implementation the hunk's Before
19+
// ends "...\nEOF" and the patch matches nothing. Ladder rung 9 on 2026-08-03
20+
// failed 24 of its 34 patches, and the tool's own "it was looking for" text
21+
// ends in EOF.
22+
func TestTrailingTextAfterTheEndMarkerIsNotContext(t *testing.T) {
23+
raw := "*** Begin Patch\n" +
24+
"*** Update File: main.go\n" +
25+
"@@\n" +
26+
" func main() {\n" +
27+
"-\tprintln(\"old\")\n" +
28+
"+\tprintln(\"new\")\n" +
29+
" }\n" +
30+
"*** End Patch\n" +
31+
"EOF\n"
32+
33+
request, err := ParsePatch(raw)
34+
if err != nil {
35+
t.Fatalf("this patch is well formed: %v", err)
36+
}
37+
if len(request.Hunks) != 1 {
38+
t.Fatalf("want one hunk, got %d", len(request.Hunks))
39+
}
40+
if strings.Contains(request.Hunks[0].Before, "EOF") ||
41+
strings.Contains(request.Hunks[0].After, "EOF") {
42+
t.Fatalf("text after the end marker became context, so the hunk asks "+
43+
"the file to contain a line no file contains:\nbefore=%q\nafter=%q",
44+
request.Hunks[0].Before, request.Hunks[0].After)
45+
}
46+
47+
// And the hunk still says what it was always for.
48+
file := "package main\n\nfunc main() {\n\tprintln(\"old\")\n}\n"
49+
patched, _, err := ApplyPatch(file, request)
50+
if err != nil {
51+
t.Fatalf("the hunk should apply: %v", err)
52+
}
53+
if !strings.Contains(patched, `println("new")`) ||
54+
strings.Contains(patched, `println("old")`) {
55+
t.Errorf("the patch did not land: %q", patched)
56+
}
57+
}
58+
59+
// TestASecondEnvelopeReopensThePatch is the control on the end marker.
60+
//
61+
// Ignoring everything after "*** End Patch" is right for trailing junk and
62+
// wrong for a run that wrapped each file in its own envelope. The second
63+
// envelope's hunks are real, and dropping them silently would lose changes the
64+
// run believes it made — which is the same failure the two-file refusal exists
65+
// to prevent, arrived at from the other direction.
66+
func TestASecondEnvelopeReopensThePatch(t *testing.T) {
67+
raw := "*** Begin Patch\n" +
68+
"*** Update File: main.go\n" +
69+
"@@\n" +
70+
"-\tprintln(\"one\")\n" +
71+
"+\tprintln(\"ONE\")\n" +
72+
"*** End Patch\n" +
73+
"*** Begin Patch\n" +
74+
"*** Update File: main.go\n" +
75+
"@@\n" +
76+
"-\tprintln(\"two\")\n" +
77+
"+\tprintln(\"TWO\")\n" +
78+
"*** End Patch\n"
79+
80+
request, err := ParsePatch(raw)
81+
if err != nil {
82+
t.Fatalf("both envelopes name the same file: %v", err)
83+
}
84+
if len(request.Hunks) != 2 {
85+
t.Fatalf("the second envelope's hunk was dropped: got %d hunk(s)",
86+
len(request.Hunks))
87+
}
88+
89+
file := "package main\n\nfunc main() {\n\tprintln(\"one\")\n\tprintln(\"two\")\n}\n"
90+
patched, _, err := ApplyPatch(file, request)
91+
if err != nil {
92+
t.Fatalf("both hunks should apply: %v", err)
93+
}
94+
if !strings.Contains(patched, `println("ONE")`) ||
95+
!strings.Contains(patched, `println("TWO")`) {
96+
t.Errorf("both changes should have landed: %q", patched)
97+
}
98+
}

internal/executor/patch_tool.go

Lines changed: 106 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ type PatchHunk struct {
4444
// Added and Removed count what this hunk does, for the size limits.
4545
Added int
4646
Removed int
47+
// Anchor is file text this hunk is inside, used only to say which of
48+
// several identical places is meant. It is never applied and never
49+
// required to match; it narrows where Before is looked for and nothing
50+
// else.
51+
//
52+
// It comes from the two forms a patch uses to name a scope: a heading on
53+
// the "@@" line, and a preceding hunk of nothing but context.
54+
Anchor string
4755
}
4856

4957
// PatchOutcome is what applying a patch did, for the record and the reply.
@@ -95,6 +103,17 @@ func ParsePatch(raw string) (PatchRequest, error) {
95103
current, before, after = nil, nil, nil
96104
}
97105

106+
// Whether the patch has already said it is over.
107+
//
108+
// It matters because an unprefixed line is accepted as context, which is
109+
// right inside a hunk and wrong after the end marker: a run that wrote
110+
// "*** End Patch" and then a stray "EOF" — the shape a shell heredoc
111+
// leaves behind, and one models reproduce — had EOF folded into the hunk's
112+
// context, so the hunk was searched for as the file's own text plus a line
113+
// no file has ever contained and could not match anything. Ladder rung 9 on
114+
// 2026-08-03 failed 24 of its 34 patches, and this is what the tool
115+
// reported it was looking for.
116+
ended := false
98117
for _, line := range lines {
99118
trimmed := strings.TrimSpace(line)
100119
switch {
@@ -103,6 +122,7 @@ func ParsePatch(raw string) (PatchRequest, error) {
103122
strings.HasPrefix(trimmed, "--- "),
104123
strings.HasPrefix(trimmed, "+++ "):
105124
closeHunk()
125+
ended = false
106126
if named := patchHeaderPath(trimmed); named != "" {
107127
if request.Path == "" {
108128
request.Path = named
@@ -111,14 +131,25 @@ func ParsePatch(raw string) (PatchRequest, error) {
111131
alsoNamed = append(alsoNamed, named)
112132
}
113133
}
114-
case strings.HasPrefix(trimmed, "*** Begin Patch"),
115-
strings.HasPrefix(trimmed, "*** End Patch"),
116-
strings.HasPrefix(trimmed, "diff --git"),
134+
case strings.HasPrefix(trimmed, "*** Begin Patch"):
135+
// Envelope, and the start of a fresh one: whatever came before is
136+
// finished and anything after this belongs to the patch again.
137+
closeHunk()
138+
ended = false
139+
case strings.HasPrefix(trimmed, "*** End Patch"):
140+
closeHunk()
141+
ended = true
142+
case strings.HasPrefix(trimmed, "diff --git"),
117143
strings.HasPrefix(trimmed, "index "):
118144
// Envelope. Carries nothing this needs.
145+
case ended:
146+
// Past the end marker. Trailing text is not part of any hunk.
119147
case strings.HasPrefix(trimmed, "@@"):
120148
closeHunk()
121-
current = &PatchHunk{}
149+
// "@@ func TestFoo(t *testing.T) {" names the scope the change is
150+
// inside, and the whole line was being discarded. Kept as an
151+
// anchor, it is what tells a "}" which "}" is meant.
152+
current = &PatchHunk{Anchor: hunkHeaderAnchor(trimmed)}
122153
case current == nil:
123154
// Text outside any hunk. Prose, or a stray line; either way it
124155
// changes nothing and saying so would be noise.
@@ -173,18 +204,37 @@ func ParsePatch(raw string) (PatchRequest, error) {
173204
"lines to add prefixed \"+\", and enough unprefixed lines " +
174205
"around them to say where the change goes")
175206
}
176-
// A hunk that changes nothing is dropped rather than refused.
207+
// A hunk that changes nothing is not applied — but it is not noise either.
208+
//
209+
// Models emit them for orientation, and refusing the whole patch for one
210+
// costs the round, so they were dropped. Dropping was too much: a block of
211+
// context immediately before a change is how a patch names the scope the
212+
// change is inside, and it is exactly what makes " }\n}" mean one closing
213+
// brace rather than every closing brace in the file. Ladder rung 9 on
214+
// 2026-08-03 sent the same patch six times in a row and was told to add
215+
// more context each time, with the context it needed in the hunk above;
216+
// 103 of that run's 109 patch failures were "matches N places".
177217
//
178-
// Models emit them for orientation — a block of context around the part
179-
// they are about to describe, or a trailing block after the last change —
180-
// and refusing the whole patch for one costs the round. Skipping it cannot
181-
// change the file, which is the only thing that makes forgiveness safe
182-
// here. Ladder rung 8 lost roughly fifteen patches to this.
218+
// So it carries forward as the next hunk's anchor instead. A context block
219+
// after the last change still anchors nothing and is still dropped, which
220+
// is the case forgiveness was added for.
183221
changing := request.Hunks[:0:0]
222+
pending := ""
184223
for _, hunk := range request.Hunks {
185224
if hunk.Added == 0 && hunk.Removed == 0 {
225+
// The nearest preceding one wins: it is the innermost scope named,
226+
// and an outer scope cannot contradict it.
227+
if strings.TrimSpace(hunk.Before) != "" {
228+
pending = hunk.Before
229+
}
186230
continue
187231
}
232+
// A heading on the hunk's own marker line is more specific than a
233+
// block that preceded it, so it is not overwritten.
234+
if hunk.Anchor == "" {
235+
hunk.Anchor = pending
236+
}
237+
pending = ""
188238
changing = append(changing, hunk)
189239
}
190240
if len(changing) == 0 {
@@ -251,6 +301,26 @@ func ApplyPatch(existing string, request PatchRequest) (string, PatchOutcome, er
251301
// exactly as before.
252302
cursor := 0
253303
for index, hunk := range request.Hunks {
304+
// The scope the hunk named, if it named one and the file has it.
305+
//
306+
// Searching from just after it is what makes " }\n}" mean this
307+
// function's closing brace rather than every function's. An anchor that
308+
// resolves to nothing is ignored rather than refused: it only ever
309+
// narrows where Before is looked for, so failing on it would turn a
310+
// stale scope name into a lost patch.
311+
from := cursor
312+
if hunk.Anchor != "" {
313+
if at := uniqueIndexIn(patched[cursor:], hunk.Anchor); at >= 0 {
314+
from = cursor + at + len(hunk.Anchor)
315+
}
316+
}
317+
if at := uniqueIndexIn(patched[from:], hunk.Before); at >= 0 {
318+
absolute := from + at
319+
patched = patched[:absolute] + hunk.After +
320+
patched[absolute+len(hunk.Before):]
321+
cursor = absolute + len(hunk.After)
322+
continue
323+
}
254324
if at := uniqueIndexIn(patched[cursor:], hunk.Before); at >= 0 {
255325
absolute := cursor + at
256326
patched = patched[:absolute] + hunk.After +
@@ -293,6 +363,32 @@ func ApplyPatch(existing string, request PatchRequest) (string, PatchOutcome, er
293363
}, nil
294364
}
295365

366+
// hunkHeaderAnchor is the file text a "@@" line names, or empty when it names
367+
// none.
368+
//
369+
// A hunk header carries up to two things after its marker: a line range, which
370+
// is bookkeeping about a file this tool does not have, and a section heading,
371+
// which is a line copied from the file and is exactly the anchor an ambiguous
372+
// hunk is missing. The range is dropped and the heading is kept.
373+
//
374+
// Empty for a bare "@@", and empty for a header that is only a range: an anchor
375+
// that is not file text would match nothing and turn a good patch into a
376+
// refusal, which is worse than the ambiguity it was meant to resolve.
377+
func hunkHeaderAnchor(line string) string {
378+
rest := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(line), "@@"))
379+
// A trailing "@@" closes a range, and everything after it is the heading.
380+
if closing := strings.Index(rest, "@@"); closing >= 0 {
381+
// One leading space is the format's separator, not indentation.
382+
return strings.TrimRight(
383+
strings.TrimPrefix(rest[closing+2:], " "), " \t")
384+
}
385+
// An unclosed range is bookkeeping and nothing else.
386+
if strings.HasPrefix(rest, "-") || strings.HasPrefix(rest, "+") {
387+
return ""
388+
}
389+
return strings.TrimRight(rest, " \t")
390+
}
391+
296392
// indentForMessage renders a hunk's target inside a refusal, bounded.
297393
func indentForMessage(text string) string {
298394
lines := strings.Split(text, "\n")

0 commit comments

Comments
 (0)