Skip to content

Validate NAIBR BEDPE records before inferring structural-variant classes - #297

Merged
pdimens merged 1 commit into
pdimens:mainfrom
thierrygosselin:fix-infer-sv-validation
Sep 9, 2026
Merged

pdimens merged 1 commit into
pdimens:mainfrom
thierrygosselin:fix-infer-sv-validation

Conversation

@thierrygosselin

@thierrygosselin thierrygosselin commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

harpy-utils infer-sv previously assumed that every NAIBR BEDPE record contained the expected number of columns and a valid orientation code. Malformed or changed input could therefore produce an uninformative IndexError or KeyError.

This change validates the input before structural-variant inference:

  • reject empty files and headers with too few columns;
  • reject malformed records with a line-specific error;
  • validate orientation values against the four supported NAIBR mappings;
  • preserve the existing handling of passing and FAIL records.

The resulting error messages identify the offending line and value, making upstream-format changes and damaged files much easier to diagnose.

Scope

This is intentionally limited to input validation and diagnostics. It does not change the structural-variant classification rules.

Summary by CodeRabbit

  • Bug Fixes
    • Added validation for empty or malformed NAIBR BEDPE input files.
    • Reports clear errors for records with insufficient columns or unsupported orientation values.
    • Preserves meaningful line endings and reports the affected input line during processing.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 8f412780-19cb-49f7-82b2-e5a029de73ad

📥 Commits

Reviewing files that changed from the base of the PR and between a7e3543 and ac1f2e1.

📒 Files selected for processing (1)
  • harpy/utils/infer_sv.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

infer_sv now validates NAIBR BEDPE headers and records before processing. It reports empty input, short rows, and unsupported orientations with click.ClickException, including the invalid record line number.

Changes

BEDPE input validation

Layer / File(s) Summary
Input validation and processing
harpy/utils/infer_sv.py
The header must contain at least seven columns. Each record must contain at least seven columns and use an orientation present in conversions. Invalid records report their line number. Parsing removes only line endings with rstrip("\r\n").

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~15–30 minutes

Merge Risk: ⚪ Minimal · up to ac1f2

The command now rejects malformed NAIBR BEDPE input with specific errors while preserving existing passing, FAIL, and SV classification behavior. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: validating NAIBR BEDPE records before structural-variant inference.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pdimens

pdimens commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Thank you for submitting this PR! I'm in favor of the validation, but I also have to ask about what use-case you have encountered that a malformed BEDPE file would be used as input for the utility? Within harpy, infer-sv exclusively works on the output produced by the sv naibr workflow.

@pdimens
pdimens merged commit 8c16af3 into pdimens:main Sep 9, 2026
15 checks passed
@thierrygosselin

Copy link
Copy Markdown
Contributor Author

Thanks Pavel, that makes sense. I mainly had two defensive edge cases in mind:

  1. BEDPE files that are truncated or edited during troubleshooting or reruns
  2. A future change to the NAIBR BEDPE output format, although unlikely

The validation is intended to give a clear line-specific error in those cases while leaving the normal sv naibr workflow unchanged.

Does that make sense?
Cheers and say Hi to Nina for me
Thierry

@pdimens

pdimens commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Yeah, that makes sense. Thanks for the helpful contribution!

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.

2 participants