Fix: implement classification for in-frame complex variants

[kunal-10-cloud]

Summary

Replaces TODO placeholder code with a classification helper function that accurately determines effect and impact for in-frame complex variants by comparing amino acid sequences. Includes position-aware detection for START/STOP codon changes and comprehensive multi-codon test coverage.
Fixes: #1198

What Changed

Added _classify_inframe_complex_effect() helper function that:

• Compares reference vs. alternate amino acid sequences
• Detects synonymous mutations, missense changes, and functionally important variants (START/STOP codon alterations)
• Assigns scientifically justified impact levels (LOW, MODERATE, HIGH) instead of UNKNOWN
• Returns properly classified variant effects consistent with existing SNP patterns
• Position-aware checks: Handles multi-amino-acid strings (e.g., "MLR", "Q*R") correctly

Added comprehensive test suite with 16 tests validating:

• All classification branches and edge cases (9 original + 4 new multi-codon tests)
• Data integrity and field preservation
• Absence of TODO strings in output
• Scientific correctness of impact assignments
• Multi-codon scenarios: START_LOST detection on first position, STOP mutations anywhere in string

Validation

• ruff linting: Passed
• mypy type checking: No issues
• Pre-commit hooks: All checks passed
• Test results: 16/16 passing (including 4 new multi-codon tests)

Technical Approach

In-frame complex variants have complete amino acid information available, enabling SNP-style classification. Position-aware logic ensures:

• Accurate START_LOST detection by checking first amino acid only (ref_aa[0] == "M")
• Accurate STOP mutations detection by checking presence of "" anywhere in string ("" in ref_aa / alt_aa)
• Reliable handling of rare multi-codon translations
• Consistent effect naming with existing code patterns

[Copilot]

Pull request overview

This PR addresses Issue #1200 by replacing the in-frame complex-variant “TODO” placeholder in veff with a helper-based classification path, and adding tests to validate the new behavior.

Changes:

• Replaced placeholder effect/impact assignment for mixed MNP+INDEL in-frame variants with _classify_inframe_complex_effect().
• Added a new unit test module covering the helper’s effect/impact outputs and basic invariants (e.g., no “TODO”, field preservation).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
malariagen_data/veff.py	Routes in-frame complex (mixed MNP+INDEL) variants through a new helper to assign non-UNKNOWN effect/impact.
tests/test_veff.py	Adds tests for the helper and basic output integrity checks.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jonbrenas]

Hi @kunal-10-cloud, as I explained in #1181 (which solves the exact same issue you are addressing here), the situation is more complex. Your solution is more complex than what could be found in #1181, granted, but it is also not accurate.

[kunal-10-cloud]

hi @jonbrenas, I have tried to simplify the approach. Can you please take a look at it and let me know how can i proceed

[jonbrenas]

Hi @kunal-10-cloud, I don't think you understood what I was saying. You basically reverted your PR to what was in #1181. That's not what I was asking at all. What I am saying is that:

1. This code is never reached so you shouldn't bother.
2. What the effects should be in such a situation is a question of biology and not of computer science or software engineering.
3. The problem is almost certainly not described properly.

All that to say, this is not an issue that you can or should try to solve and so this PR has no reason to exist.

[kunal-10-cloud]

Hi @kunal-10-cloud, I don't think you understood what I was saying. You basically reverted your PR to what was in #1181. That's not what I was asking at all. What I am saying is that:

1. This code is never reached so you shouldn't bother.
2. What the effects should be in such a situation is a question of biology and not of computer science or software engineering.
3. The problem is almost certainly not described properly.

All that to say, this is not an issue that you can or should try to solve and so this PR has no reason to exist.

Hi @jonbrenas i understood your point and i was aware of this thats why in the comments i mentioned the same thing as well and asked how to proceed irrespective of that really thanks for the description and taking out your time.
Will close this issue for now can you please review these PR's instead

#1190(Approved) #1199(review) #1188(review)