Skip to content

fix: stop the validator refusing patches git would have applied - #145

Open
thedancingdeveloper wants to merge 2 commits into
mainfrom
fix/validator-rejects-valid-patches
Open

fix: stop the validator refusing patches git would have applied#145
thedancingdeveloper wants to merge 2 commits into
mainfrom
fix/validator-rejects-valid-patches

Conversation

@thedancingdeveloper

Copy link
Copy Markdown
Contributor

Found by pointing the harness at the real NGMS backlog instead of a toy repo.
Three ways a well-formed patch was refused before git ever saw it.

1. A patch that creates a file

T28 patch_malformed — line 141: line inside a hunk starts with 'n',
                       not ' ', '+' or '-': 'new file mode 100644'

new file mode 100644 is what git diff emits for adding a file — the
ordinary way to add a module. It was not in _BETWEEN_HUNKS, so the validator
read it as corrupt body content. deleted file mode, similarity index,
dissimilarity index, rename from/to and copy from/to were missing for
the same reason.

2. A hunk header that over-declares, mid-diff

Both detection sites marked the problem fatal, contradicting
PatchProblem's own docstring:

fatal separates damage no rung of the ladder can repair … from damage that
IS routinely repaired, notably hunk headers whose line counts are wrong.
Only the first kind is worth refusing over; refusing the second would throw
away work --unidiff-zero rescues every day.

They are now reported and not refused. The reasoning is the same one that made
the end-of-input case conditional: here another hunk or another file follows,
so nothing was cut off — the counts are simply wrong, and recount_hunks
derives them from the body. The end-of-input case is untouched and still
refuses unless both shortfalls match, because there a genuine truncation is
possible.

3. A patch with no hunks at all

A rename, a delete or a mode change is a complete patch with nothing to hunk.
The no-hunk check exists to catch a model answering in prose, so it now
requires a real file header before refusing.

Live effect

Two NGMS items that died at parse now reach apply and fail on what git
actually says — Cargo.toml: patch does not apply instead of corrupt patch at line 53. They still fail, for a different and real reason (see the follow-up
issue on repository context), but the harness is no longer discarding patches
on its own account.

Six new tests: creating a file, deleting one, a rename, that the accepted patch
really applies, and that a \ No newline at end of file marker does not
shorten a recount. Suite, ruff and mypy . green.

sprooty added 2 commits August 4, 2026 00:53
…e CLI

Chains shipped in the CLI and in the stored role map, and `RoleRoute` -- the
only way to configure a deployed pod -- could not express one. Found while
repointing Node B: the models an operator had chosen could be set locally and
not on the machine that runs the work, which makes the feature unshipped where
it matters.

`models` names the chain in preference order; `model` stays the preferred one
and is filled in from the first when omitted, so every existing map, client and
reader that knows only `model` is unaffected. Giving both is allowed and
checked: a route whose two fields contradict each other behaves differently
depending on which one a reader consults, so it is refused with the mismatch
named rather than silently resolved.
Three ways a well-formed patch was called corrupt, all found by pointing the
harness at a real backlog rather than a toy repository.

**A patch that creates a file.** `new file mode 100644` is what git emits for
the ordinary way to add a module, and it was not in the list of lines allowed
between hunks -- so the validator read it as body content and refused with
`line inside a hunk starts with 'n'`. `deleted file mode`, `similarity index`,
`rename from`/`to` and `copy from`/`to` were missing for the same reason.

**A hunk header that over-declares, mid-diff.** Both places that detect it
marked the problem fatal, which contradicts what `PatchProblem` documents
about itself: line counts that are wrong are "routinely repaired, notably
hunk headers whose line counts are wrong... refusing the second would throw
away work `--unidiff-zero` rescues every day". They are now reported and not
refused, because another hunk or another file follows -- so nothing was cut
off, and `recount_hunks` derives the right counts from the body. The
end-of-input case is unchanged: there, a genuine truncation is possible, so it
is still refused unless both shortfalls match.

**A patch with no hunks at all.** A rename, a delete or a mode change is a
complete patch with nothing to hunk. The no-hunk check exists to catch a model
answering in prose, so it now looks for a real file header before refusing.

Live effect on NGMS: two items that failed at `parse` now reach `apply` and
fail, honestly, on what git actually says -- `Cargo.toml: patch does not
apply` rather than `corrupt patch at line 53`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant