Variant record page: VCF-backed tables, strain filter, and section cleanup - #215
Merged
Conversation
Rebuilds the legacy SNP record's per-strain and per-country tables on the Variant record, reading the published merged.ann.vcf.gz at request time instead of loading anything. Three deliverables: a VariantStrains table (one row per VCF sample), a VariantCountrySummary table (one row per country, country-having samples only), and a port of the legacy record-page strain filterParam. Two findings drove the shape. Record-page tables backed by a processQuery are NOT cached - SingleRecordAnswerValue routes them to TableFieldProcessQueryResult, which calls getUncachedResults(). So there is no cache table, no per-page-view DDL, and nothing to evict. This is specific to record tables; a process query reached any other way caches unconditionally, which is why deliverable C stays SqlQuery-backed. And the frequency semantics have to be taken from processSequenceVariations.jl rather than reinvented: chromosome-weighted denominators (not strain counts), IUPAC ambiguity codes for hets, AO/(RO+AO) read frequencies, and coverage-filled reference calls that carry no AO/RO and must render as 100%. Getting any of these wrong yields numbers that silently disagree with the attributes shown in the overview panel on the same page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Confirmed against genomicsdb_071n. The grouping attribute is provider_label ["country"], keyed on the label rather than the VAR_<hash> stable_id, which is a digest of the label and site-specific. Legacy used geographic_location, and porting that verbatim would have been wrong: both attributes exist in the P. falciparum dnaseq study, but country covers 536 of 537 samples against geographic_location's 111. Also records that 14 of the 62 dnaseq studies carry no country attribute at all, and that no fallback is worth building - 13 of them have no geography attribute of any kind, and the one exception (P. yoelii 17X) has a single populated value of "Lowlands West and Central Africa" across 6 samples. Table B renders empty for those organisms rather than erroring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Eleven tasks, TDD throughout, over the design in 2026-08-11-variant-strain-tables-design.md. Task 1 retires both htsjdk risks before anything depends on them: whether the version-less dependency resolves, and whether VCFCodec tolerates this file's under-declared header. Tasks 2-5 build the WDK-free parsing and composition layer against a fixture that reproduces those header quirks on purpose. Tasks 6-8 add the two tables and their model wiring; 9-10 port the record-page strain filter; 11 verifies the whole instance, including an organism whose study has no country attribute. Every command is concrete: the locus SQL was run against genomicsdb_071n, the individuals.txt rows carry the verified 14-field tab layout, and the build uses wb full (a Java plugin changed) followed by wb ontology (individuals.txt changed) - wb model alone would silently miss both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The stated formula gave the alt branch AO[altIndex] and the reference branch sum(AO) - deliberately asymmetric, since reference support is measured against all non-reference reads. But the Step 5 code implemented both branches as sum(AO), so on a multi-allelic record a sample's read frequency would be diluted by reads supporting an alt it does not carry. Caught in spec review of Task 3, where the implementer had transcribed the code faithfully and the fixture - biallelic, like every real record - could not expose the disagreement. Latent rather than live: measured 548,040 alt calls across 40,000 loci of the real merged VCF with no record carrying more than one ALT and no sample carrying more than one nonzero AO, because write_vcf_entry emits one record per unique alt. Corrected anyway, and the measurement recorded, because a formula that is only accidentally right is a trap for whoever reads it next. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two corrections found in code review of Task 3, both defects in these documents rather than in the implementation. intsOf dropped '.' slots instead of preserving them, which compacts the array and misaligns every later index: AO="1,.,4" became [1,4], so a sample called for alt2 would read alt3's support as its own. That silently defeats the indexed lookup in readFrequency, which exists precisely for the multi-allelic case. It now preserves slot positions with a NO_VALUE sentinel, while still reporting a wholly-'.' value as absent, which is what the coverage-filled discrimination keys on. The design doc claimed a coverage-filled row has "every other FORMAT field '.'". Only the original FORMAT set is blanked - write_vcf_entry appends CA and DFS after the fill, so a filled row carries both and neither can be used to detect one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three review-driven changes: the SQLException catch narrowed to 42P01, the EDA-suffix guard split so a null suffix yields an empty map rather than an exception, and vcfPath validating its inputs instead of building 'build-null'. The plan's code snippets are left in place but marked non-authoritative for this task - git is. Recording the divergence rather than silently letting the plan drift, since a plan that disagrees with the code is what produced two of the defects found so far. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Backed by a processQuery reading the merged VCF at request time. Record-page table queries run uncached, so this persists nothing and needs no data load. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A merged genotype is no longer one real VCF genotype once a locus spans several merged VCF records (see the ApiCommonWebService fix on this same branch: MergedVcfReader now merges every record at a locus, and a sample with contributing alts on more than one record gets a slash-joined GT that was never a literal call). Requested by the user, and doubly right now that the value can be a synthetic join rather than a raw "0/1". Follows the existing internal="true" inReportMaker="true" idiom used on TranscriptProducts.gene_source_id: hidden on the record page, still available in the report maker and file downloads.
TranscriptProducts enumerates per-transcript products for every possible alt allele, so on a multi-record locus it shows rows no strain can reach. Drop the 'record' scope (keeping 'download') so it stops rendering on the record page while remaining available in the report maker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
At a biallelic SNP - the overwhelming majority - Other Allele is blank in every row, and Minor Allele is blank at a monomorphic country. Nothing said that a blank means "this country has no such allele", so an entirely empty column read as missing data. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A filterParam has to live in a question, so VariantAlignmentForm is a query returning no rows whose only job is to host the params - the same device the legacy SnpAlignmentForm used, rewritten here because the snp block is unimported dead code. Points at SamplesMetadataByStudy rather than the WithRef variant: this filter selects VCF sample columns, and the reference strain is not one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
organismSinglePick defaults to quote="true", so WDK quoted the value and the '$$organismSinglePick$$' inside the vocab query VariantVQ.EdaSampleTableSuffix quoted it again, producing ''pfal3D7'' and a 500 from the searches service - the record page's strain filter rendered as a silently empty section. The missing queryRef also left the default coming from the unscoped organism vocabulary, giving an abbrev where that query matches on a scientific name. Both attributes now match the four VariantsBy* searches in this file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reparent SNP Alleles, Indel Alleles, and Strain Statistics from the DNA polymorphism subheading up to Genetic Variation, so the Variant record's sections sit together rather than nested a level down. The GeneVariation* categories keep their DNA polymorphism parent. Set nine attributes the record summary already renders at the top of the page to record-internal, so they appear once rather than twice: Variant Type, Coding, Reference Strain, Gene ID(s), both Effects columns, both Most Severe Impact columns, and Minor Allele Frequency. They keep their download scope. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Set Allele, Sequence, Position, and Variant Call Set to record-internal - all four are already shown in the record summary at the top of the page. Rename the strain filter section to 'Explore Sample Details and Multiple Sequence Alignment Tool'. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Location, NCBI Taxon ID, and Organism are the last three record-scoped attributes in that category and all three are already in the record summary directly above, so the section now has nothing visible and disappears. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mirrors the Gene record's SnpsGbrowseUrl: the same geneticVariationTracks endpoint and the same <abbrev>_dnaseq_merged_short_variants track, windowed on the variant position +/- 1kb with the position itself highlighted. The coordinates live in a new VariantBrowser attribute query rather than in VariantTuning, which also backs result grids - folding the organism join into that query would charge every row of every search for two attributes only the record page reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every direct child of Genetic Variation had an empty displayOrder, so section order was implicit. Make it explicit, with the genome browser first: browser, SNP Alleles, Indel Alleles, Strain Statistics, Predicted Effects, Strains / Samples, Country Summary, the strain filter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two JBrowse URL attributes were marked internal="true", so the service never sent them to the client; the Gbrowse wrapper read undefined and the iframe resolved 'undefined&tracklist=0...' as a relative app route, giving a 404 on record-types/variation/records. They stay off the page body because nothing in individuals.txt references them, not because they are internal. Section order: WDK renders sub-categories before leaf attributes, so displayOrder=1 only made the browser first among the leaves - it still landed below Strain Statistics. Give it its own Genome Browser category ordered ahead of SNP Alleles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The record page only requests attributes present in the category tree, so with no rows at all the client received neither URL and the iframe src was undefined. The gene page does exactly this for snpsJbrowseUrl and snpsJbrowseFullUrl: record-internal means fetched but never rendered, which is what these need. Clearing internal="true" in the XML was necessary but not sufficient. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Genomic Location is a top-level section above Genetic Variation and had no visible attributes left after the summary-duplicate cleanup, so putting the browser there both places it above everything and removes the extra Genome Browser category, which was nesting a section inside a section. Window is now the variant position +/- 4kb. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
variantRecordUrlFxn rebuilds a Short Variant record id from a VCF feature: the merged VCF's ID column is '.' on every row, so the id comes from the locus. VCFFeature sets start = POS-1, and VariationAttributes.location IS the VCF POS, so start+1 recovers it - indels included, verified against real records. variantRecordTitleFxn carries the link to the full record page in the dialog title, which dijit renders as HTML. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A subclass rather than a flag on VcfTrackConfig, which is shared with addVCF's per-sample ebi_VCF tracks - those have no Variant records, so an unconditional onClick would link them to a 404. Left-click opens the Short Variant record (Overview + Predicted Effects) in an iframe dialog, with a link to the full record page in the dialog title. menuTemplate is deliberately not set: Util.deepUpdate merges arrays index-wise, so any menuTemplate here would silently clobber the leading entries of JBrowse's default menu - whose 'View details' is the raw-VCF escape hatch we want to keep. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
c8baece added the glyph/track_type_display arg hooks and raised the rendering ceilings, all for the merged short-variant track. VariantTrackConfig now owns those, so the base class goes back to serving the per-sample ebi_VCF tracks it was written for. The string->number fix for chunkSizeLimit stays: that was a real bug independent of the merged track. BEHAVIOUR CHANGE: VectorBase's per-sample ebi_VCF tracks lose maxFeatureScreenDensity 50 and revert to JBrowse's default 0.5. On a single-sample VCF that is the behaviour they had before 2026-08-10, but if anything has come to rely on those tracks drawing while zoomed out, this is the commit that changed it back. fetchSizeLimit disappears too, but unlike the density change that is a pure return to pre-c8baecec8 behaviour: per-sample VCFs never set it before. Note for bisecting: this commit alone leaves the merged short-variant track broken. addMergedShortVariants still constructs VcfTrackConfig directly and still passes glyph/color/track_type_display, which this commit stops honouring, so the merged call set would render mislabeled and with a 10MB chunk ceiling. The next commit points that caller at VariantTrackConfig, which is where those values now live. Do not build from this commit in isolation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The subclass owns the glyph, colour and track type now, so the caller stops passing them - one way to configure each thing, not two. This also completes the previous commit: VcfTrackConfig stopped honouring those args, and this points the caller at the class that does. 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
1 participant
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.
Model wiring for the two VCF-backed Variant record tables (plugins in VEuPathDB/ApiCommonWebService#21), the record-page strain filter, and a tidy-up of the page's organization.
Tables
VariantStrains("Strains / Samples") andVariantCountrySummary("Country Summary") — processQueries plus record tables and ontology rows. Country Summary shows only samples that have a country; its Strains counts therefore do not sum to Called Strain Count, and the column help says so. "Other Allele" is rank 3 rather than "everything else", so a 4-allele locus sums to less than 1 — also stated in the help.Strain filter
A
filterParammust live in a question and a record page has none, soVariantAlignment.VariantAlignmentFormis a query returning zero rows whose only job is to host the params — the device the legacySnpAlignmentFormused, rewritten because the snp block is unimported dead code. It points atSamplesMetadataByStudy, not the...WithRefvariant the search filters use: this filter selects VCF sample columns and the reference strain is not one.Worth reading before copying: the
organismSinglePickparamRef needs bothquote="false"and a scopedqueryRef. Without the first, WDK quotes the value and the already-correct'$$organismSinglePick$$'inside the vocab query quotes it again →''pfal3D7''and a 500 from the searches service. Without the second, the default comes from the unscoped organism vocabulary and is an abbrev where that query matches on a scientific name. Everything builds and loads clean either way; the record page just renders a silently empty section.Page cleanup
SNP Alleles, Indel Alleles, and Strain Statistics promoted from the DNA polymorphism subheading to Genetic Variation (the
GeneVariation*categories keep their old parent). Attributes already shown in the record summary set torecord-internalso they appear once — including all of Genomic Location, which now has nothing visible and disappears.TranscriptProductsalso hidden: it enumerates products for every possible alt allele, so at a multi-record locus it shows rows no strain can reach. All keep their download scope.🤖 Generated with Claude Code