BED reporter and feature provider for strain genomic segments - #307
Merged
Conversation
The strain group in PATTERN was [^:_]+, excluding underscore. That was
written on a false premise: that no strain name contains an underscore.
Re-measured against the live database, the opposite is true -- of 6,119
strain names carrying indel data, 1,494 (24%) contain an underscore
(1_01_01, Af293_resequence2, China_LZCH-36, USGS_28834_1_NV,
AFIS_13708_CDC-14), and 0 contain a colon. So parse() was rejecting
roughly a quarter of all legitimate primary keys.
Relax the strain group to [^:]+. Colon is the only true delimiter, which
also aligns the Java parser with the SQL side (which already splits on
':' alone; 1_02_01:bcin_chr_1:1-100000:f resolves correctly there).
The old constraint was also justified by a claim that it kept
getStrainSeqId() reversible. It never did: refSeq contains underscores
too, so A_B + C is indistinguishable from A + B_C regardless of how the
strain grammar is narrowed. Replace that comment with one stating the key
is opaque and one-way -- it exists only to match the dnaseq FASTA defline
{sample}_{chrom_name}; consumers needing the strain parse it out of the
primary key, where ':' delimits unambiguously.
All coordinate and strand validation is unchanged. Adds five tests over
the real strain names above, which fail on the old regex and pass on the
new one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
StrainSegmentFeatureProvider emits one BED6 line per strain genomic segment. chrom comes from the strain_seq_id attribute rather than being recomputed, so the model stays the single source of truth for the strain consensus FASTA key; name carries provenance (both coordinate systems) via DeflineBuilder, honouring RequestedDeflineFields, and is the bare primary key unless deflineType=full. It rejects an inverted strain interval (strain_end < strain_start), which StrainSegmentAttributes.Coords deliberately leaves unclamped so a segment lying inside a deletion is visible here. Nothing upstream rejects it, and reaching a FASTA lookup with one is a silent wrong answer. A strain_start below 1 is rejected for the same reason: BedLine would emit a negative chromStart. The validation is a static seam so it is unit testable without a loaded WDK model; the test covers the live inverted case 366.1:Pf3D7_10_v3:331757-331757:f -> 331658..331604 (verified in genomicsdb). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cross-check the BED chrom column against the primary key. chrom still comes from the strain_seq_id attribute -- the model stays the source of truth -- but it is now compared to StrainSegmentId.getStrainSeqId() and a mismatch throws, naming both keys and the primary key. Today they cannot disagree; the check exists because if the attribute query is ever rewritten to source the strain from somewhere other than split_part, the failure is a BED line on the wrong contig with no error at all. It is also the first production consumer of getStrainSeqId(), whose javadoc already promises it is the chrom column. Test the accept side of the strainStart boundary (1-1): '<' silently becoming '<=' passed every existing test while breaking every segment starting at position 1. Note at the call site that validating before DeflineBuilder and BedLine.bed6 is load-bearing -- neither inspects its arguments. Route organism through requiredStringAttribute like the other three required attributes, so a NULL fails naming the primary key instead of rendering as an empty defline field. Docs/tests: name BedReporter.configure/getFieldsByName as the actual validator of the declared record class and attributes (the provider only declares them); list the accepted deflineFields vocabulary, which nothing else enumerates; stop claiming Task 7 end-to-end coverage in the present tense; factor the reject boilerplate into an assertIntervalRejected helper that still asserts on message content, following StrainSegmentIdTest. Co-Authored-By: Claude Opus 5 <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
2 participants
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.
The Java half of the internal
strain-genomic-segmentrecord: emits one BED line per segment, in the strain's consensus-sequence coordinates.Companion PR: VEuPathDB/ApiCommonModel#209 (record class, search, coordinate-conversion queries, and the
<reporter>registration that namesBedStrainSegmentReporterfrom this PR). Neither works without the other. Design spec and the end-to-end test report live in that PR.What's here
util/StrainSegmentId.java— the single parse/format authority for the primary key<strain>:<refSeq>:<refStart>-<refEnd>:<f|r>. Validates in the private constructor, so no instance can exist in an invalid state.feature/StrainSegmentFeatureProvider.java— reads the strain coordinates as attributes and emits the BED fields.BedStrainSegmentReporter.java— three-line wiring, same shape asBedDynSpanReporter.The two columns
chromis hard-constrained: it must equal<strain>_<refSeq>, the key into the strain consensus FASTA. Read from thestrain_seq_idattribute so the model stays the source of truth, then cross-checked against the key recomputed from the primary key — so a future divergence fails loudly instead of silently naming the wrong contig. Verified byte-exact against real FASTA deflines.nameis free — underdeflineFormat=QUERYONLYseqret emits'>' + nameverbatim, so it becomes the FASTA defline. Carries provenance; stays bare unless the caller passesdeflineType=full.The rejection this class exists to perform
The attribute query deliberately leaves
strain_start/strain_endunclamped, so a segment inside a deletion reportsstrain_end < strain_start. Nothing upstream rejects it. This provider rejects three conditions, all beforeDeflineBuilder/BedLine.bed6sees them, each naming the primary key and the offending values:strain_end < strain_start— the deletion inversion;strain_start < 1— would emitchromStart = -1;strain_seq_iddisagreeing with the key computed from the PK.Validation order at the call site is load-bearing and commented as such: neither
DeflineBuildernorbed6inspects its arguments.Running the tests
This module inherits surefire 2.12.4, where
-Dtest=A+Bmatches zero tests and still reportsBUILD SUCCESS. Use a comma and read the counts:Also: no
wbtarget compilesApiCommonWebsite/Model. After editing here runbld ApiCommonWebsite/Modelbeforewb model, or the model build fails with a misleadingImplementation class for reporter 'bed' … cannot be found.