fix: recount a hunk header the model miscounted, rather than discarding it - #143
Merged
Merged
Conversation
…ng it An implementer produced a byte-for-byte correct body under `@@ -1,10 +1,21 @@` for a nine-line file — one line too many on each side, because it counted the trailing newline. `git apply` parses by the declared counts, reaches the end of input a line early, and reports `corrupt patch`. No rung of the ladder helped: tolerance needs something to be tolerant *with*, and the parser has already run out of input. The item failed, throwing away the planner's work and an attempt. This happened on four consecutive runs of the same item, across two models. It is a common model error, not one bad reply. The counts are a derivable property of the body, so recomputing them guesses at nothing. That is what separates this from a `-0,0` header against a file with content (#133), where the header carried the only statement about *where* the lines belong and there was nothing to recompute — refused there, repaired here. Recounting is tried as a second variant of each rung, after the patch as written, so a correct diff is still applied exactly as the model produced it. Two faults wore one symptom, and they are now told apart. A header that over-counted trailing context is short by the SAME amount on both sides, because a context line counts on each; its body is complete, and it is repaired. A reply genuinely cut off mid-hunk loses a mix of `+`, `-` and context, so the shortfalls differ — recounting that would make the header agree with a partial body and land, say, a deletion whose replacement never arrived. That stays fatal. Also fixes the diagnostic, which said "the reply was truncated" for a reply that was complete at 266 output tokens. It sent me looking for a token limit that did not exist, and I filed an issue against the wrong thing before reading the preserved patch. Live: the three-item backlog that had been failing 2/3 for four runs now completes 3/3, one of them via `git apply (recounted)`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #142.
What was actually wrong
The implementer produced this — complete, correct, applies-in-spirit:
test_calc.pyhas 9 lines; the header declares 10. The body supplies 9old / 20 new against a declared 10 / 21 — one too many on each side, because
the model counted the trailing newline. The patch is 38 lines and git reports
corrupt patch at line 39: it parsed by the declared counts and ran out ofinput.
No rung rescued it.
--unidiff-zerofails identically to strictgit apply—tolerance needs something to be tolerant with, and the parser has already hit
EOF. So a correct body was discarded over arithmetic, costing the planner call,
the implementer call, and an attempt.
Four consecutive runs, two different models. A common model error.
The fix, and why it is safe
Recompute the counts from the body. They are a derivable property of it, so
this guesses at nothing — which is precisely what separates it from #133, where
a
-0,0header carried the only statement about where the lines belonged andthere was nothing to recompute. Refused there; repaired here.
Recounting runs as a second variant of each rung, after the patch as written,
so a well-formed diff is still applied byte-for-byte as the model produced it.
Telling two faults apart
They wore one symptom. The rule is principled rather than a threshold:
shortfall is equal on each. Body complete. Recountable.
+,-and context, so the shortfallsdiffer. Recounting would make the header agree with a partial body and
apply, say, a deletion whose replacement never arrived. Stays fatal.
The existing
TRUNCATED_DIFFfixture is exactly that dangerous shape and isstill refused — its test now asserts the distinction rather than the wording.
Also: the diagnostic lied
It said
the reply was truncatedfor a reply that was complete at 266 outputtokens. That sent me looking for a token limit that did not exist, and I filed
#142 against the wrong cause before reading the preserved patch. It now states
what is true: how many lines short, on which side.
Result
The backlog that failed 2/3 on four consecutive runs now completes 3/3, one
item via
git apply (recounted). Tests pass independently on each branch (4, 4and 6). Suite, ruff and
mypy .green.