fix: show the implementer the repository it is patching - #136
Merged
Conversation
The prompt asks for a unified diff that "applies cleanly at the repository root". `context_provider` defaulted to `lambda _record: ""` and the CLI never passed one, so it asked for that while showing the model nothing at all. A model cannot write context lines for a file it has not seen. So it wrote hunks with none, declaring `@@ -0,0` against files that were not empty; git apply refuses those, --unidiff-zero accepts them and inserts at line one, and code landed above module docstrings and imports with every check still green. That is the root cause behind #133, and it explains the shape of every diff this path has ever produced. Two parts: **Context.** A default provider that supplies the tracked file listing, the files the brief names in full, and as much of the rest as a byte budget allows, smallest first. Binary files are listed but not read. **Read it from the base, not the tree.** The working tree still holds the previous item's branch when the implementer is called -- the branch for this item is not cut until the apply step -- so reading the tree showed the model a file its patch would never meet. The base ref is now resolved before the implementer runs and the context is read from it with `git show`, which is correct by construction and touches no checkout. Live, on a three-item backlog with a real endpoint: 0/3 completed before, **3/3 after**, each on its own branch with its tests passing, the stacked item correctly based on its dependency, and the module docstring still the first thing in the file.
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 #135. Substantially mitigates #133.
The bug
IMPLEMENT_PROMPTasks for a diff that "applies cleanly at the repositoryroot".
context_providerdefaulted tolambda _record: "", andagent-harness runnever passed one — soRepository context:was alwaysempty and the model was asked to patch files it had never seen.
It cannot write context lines it does not know, so it wrote hunks with none:
git applyrefuses that against a non-empty file.--unidiff-zeroaccepts itand inserts at line 1 — which is how
multiplyended up above the moduledocstring, and two tests above
from calc import add, subtract, with pytestgreen throughout.
Two parts to the fix
Show it the repository. A default
context_providergiving the trackedfile listing, files the brief names in full, then the rest smallest-first
within a 60k budget. Binary files are listed, never read.
Read from the base ref, not the working tree. This one is subtle and was
found by the first fix not working. The tree still holds the previous item's
branch when the implementer is called — the branch for this item is not cut
until the apply step — so the model was shown a file its patch would never
meet, and every patch failed to apply. The base is now resolved before the
implementer runs and read via
git show <base>:<path>. Correct byconstruction, and it touches no checkout.
Live result
Same three-item backlog, same endpoint, across the three fixes in this series:
Each item on its own branch, tests passing on each independently (3, 4 and 4
tests), the stacked item correctly based on its dependency, and
calc.pyending up exactly as a human would write it — docstring first, functions in
order.
On #133
Not closed. With real context the model produces proper diffs and the first
rung of the ladder succeeds, so
--unidiff-zerostops being reached inpractice. The rung can still misplace a zero-context hunk if one arrives; that
remains worth addressing on its own terms.
Tests: four new, covering the listing reaching the implementer, a named file
being included whole under a tight budget, an empty repository, and binary
files being listed but not read. Suite, ruff and
mypy .green.