Stop a 'within' mutation from steering a neighboring deletion in NORMALIZE - #423
Merged
Merged
Conversation
…ALIZE A mutation tagged within=<MOB id> has a POSITION in a pseudo-coordinate space -- the MOB's position plus an offset into the newly inserted element -- so it occupies zero reference bases while freely colliding with real reference coordinates downstream of the MOB. normalize_and_annotate_tandem_repeat_mutations skipped such a mutation from being shifted, but its 'goto next_mutation' still assigned it to last_mut, so that pseudo-coordinate bounded the next deletion's shift. On a real curated LTEE clone this moved DEL 1270157 535 to 1270063, 94 bases outside its equivalence window [1270157, 1270234], and the output then failed APPLY because the deletion had swallowed the MOB it was backing off from. The sister clone, whose only difference is not carrying that within SNP, normalized the same authored line to 1270234 -- one shared ancestral event with two coordinates, which reads downstream as a homoplasy that never happened. Four changes: - A 'within' mutation is now skipped entirely and never becomes last_mut. The mutation it is within is itself in the list and supplies the real barrier at real coordinates. 'no_normalize' keeps the old path: those positions are genuine reference coordinates. - The DEL and INS back-off fires only when the shift is what created the overlap. The comment always said "did we get shifted into the position of the next mutation?", but the test was plain interval overlap, evaluated even when nothing moved. An interval that already covered last_mut in the input got pushed LEFT of where the author put it, by a distance that tracks last_mut's coordinate. Requiring the pre-shift interval to end before last_mut also bounds the result from below. - The APPLY self-check applies the normalized diff instead of re-parsing the input file. It was comparing the input against itself, so "Failed APPLY test" was unreachable and the non-zero exit added alongside it could never fire. It now also re-validates the output, which is what names this failure precisely: a shift can make the diff ambiguous even when the two applied sequences still agree. - NORMALIZE validates its input the way APPLY does. Normalizing an ambiguous diff is not meaningful -- the orderings give different genomes and normalization has to pick one -- and accepting it silently is how a bad coordinate got minted from a bad input. -x skips both checks. Adds the first two tests for NORMALIZE: gdtools_normalize_1 pins the within case (it emits DEL 19800, 230 bases left, without these changes) and gdtools_normalize_2 asserts the ambiguous input is rejected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
A mutation tagged within= has a POSITION in a pseudo-coordinate space -- the MOB's position plus an offset into the newly inserted element -- so it occupies zero reference bases while freely colliding with real reference coordinates downstream of the MOB. normalize_and_annotate_tandem_repeat_mutations skipped such a mutation from being shifted, but its 'goto next_mutation' still assigned it to last_mut, so that pseudo-coordinate bounded the next deletion's shift.
On a real curated LTEE clone this moved DEL 1270157 535 to 1270063, 94 bases outside its equivalence window [1270157, 1270234], and the output then failed APPLY because the deletion had swallowed the MOB it was backing off from. The sister clone, whose only difference is not carrying that within SNP, normalized the same authored line to 1270234 -- one shared ancestral event with two coordinates, which reads downstream as a homoplasy that never happened.
Four changes:
A 'within' mutation is now skipped entirely and never becomes last_mut. The mutation it is within is itself in the list and supplies the real barrier at real coordinates. 'no_normalize' keeps the old path: those positions are genuine reference coordinates.
The DEL and INS back-off fires only when the shift is what created the overlap. The comment always said "did we get shifted into the position of the next mutation?", but the test was plain interval overlap, evaluated even when nothing moved. An interval that already covered last_mut in the input got pushed LEFT of where the author put it, by a distance that tracks last_mut's coordinate. Requiring the pre-shift interval to end before last_mut also bounds the result from below.
The APPLY self-check applies the normalized diff instead of re-parsing the input file. It was comparing the input against itself, so "Failed APPLY test" was unreachable and the non-zero exit added alongside it could never fire. It now also re-validates the output, which is what names this failure precisely: a shift can make the diff ambiguous even when the two applied sequences still agree.
NORMALIZE validates its input the way APPLY does. Normalizing an ambiguous diff is not meaningful -- the orderings give different genomes and normalization has to pick one -- and accepting it silently is how a bad coordinate got minted from a bad input. -x skips both checks.
Adds the first two tests for NORMALIZE: gdtools_normalize_1 pins the within case (it emits DEL 19800, 230 bases left, without these changes) and gdtools_normalize_2 asserts the ambiguous input is rejected.