Skip to content

Catch a hand-edited status even when the primitives also moved - #379

Open
Chessing234 wants to merge 1 commit into
teorth:mainfrom
Chessing234:fix/validate-detect-hand-edited-status
Open

Catch a hand-edited status even when the primitives also moved#379
Chessing234 wants to merge 1 commit into
teorth:mainfrom
Chessing234:fix/validate-detect-hand-edited-status

Conversation

@Chessing234

Copy link
Copy Markdown
Contributor

scripts/validate.py explains that a stale status on a branch is harmless, but a contributor editing status directly is not, because regeneration silently reverts it. The check only fired when status moved and both informal_status and formal_status stayed put, so it misses the case where a PR edits a primitive and hand-edits status in the same change.

That is the likely shape of the mistake in practice: someone updates informal_status and helpfully "fixes up" status to match what they think it should be. If their value disagrees with the derived one, it passes validation and is thrown away on merge.

Reproduced against a one-problem base file (informal_status: open, status: open):

case branch content before after
A status hand-edited, primitives untouched ❌ error ❌ error
B informal_statusdisproved, status hand-edited to proved ✅ Validation OK ❌ error
C informal_statusdisproved, status left stale notice, exit 0 notice, exit 0
D informal_statusdisproved, status correctly updated silent, exit 0 silent, exit 0

Case B is the gap; C and D confirm the stale-but-untouched path and the correct path are unchanged, so this does not start rejecting ordinary edits.

By the time control reaches this branch status already disagrees with the derived value, so the only remaining question is whether the PR touched status at all. Dropping the two primitive-equality conditions is what expresses that.

Also checked for false positives on the real file: validate.py --base over the current data/problems.yaml (1217 problems) against main~5 and against main itself both report ✅ Validation OK.

The derived-status guard only fired when `status` moved and both
`informal_status` and `formal_status` stayed put. A PR that edits a
primitive and also hand-edits `status` to a value that disagrees with the
derived one therefore passed validation with a notice, and regeneration
then silently threw the hand-edit away - exactly the outcome the check
exists to prevent.

By the time we reach this branch `status` already disagrees with the
derived value, so the only question left is whether this PR touched
`status`. If it did, the edit is a hand-edit; if it did not, `status` is
merely stale on the branch, which stays a notice.
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