Skip to content

SCRUM-5953 Split allele_variant into separate allele and variant categories#1546

Merged
oblodgett merged 1 commit intostagefrom
SCRUM-5953
Apr 1, 2026
Merged

SCRUM-5953 Split allele_variant into separate allele and variant categories#1546
oblodgett merged 1 commit intostagefrom
SCRUM-5953

Conversation

@oblodgett
Copy link
Copy Markdown
Member

Summary

  • Rename Category.ALLELE_VARIANT to Category.ALLELE, add Category.VARIANT
  • Add VARIANT related data links in SearchService.addRelatedDataLinks for Gene, Disease, and Model
  • Update VariantSearchResultConverter to include species abbreviation in gene names (e.g., "Brca1 (Mmu)")
  • Add Species object to NCBITaxonTerm in VariantSummaryConverter for HTP variant species info

Test plan

  • Search for "brca1" and verify variant_search_result appears in relatedData on gene results
  • Verify allele search results still work correctly with renamed category
  • Run variant indexer and confirm gene names include species abbreviation

…nd variant categories

- Rename Category.ALLELE_VARIANT to Category.ALLELE, add Category.VARIANT
- Add VARIANT related data links in SearchService for Gene, Disease, Model
- Update VariantSearchResultConverter to include species abbreviation in gene names
- Add Species object to NCBITaxonTerm in VariantSummaryConverter for HTP variants
@oblodgett oblodgett requested a review from a team as a code owner April 1, 2026 17:24
@claude
Copy link
Copy Markdown

claude bot commented Apr 1, 2026

Code Review

The PR cleanly splits allele_variant into separate allele and variant categories. The changes are mostly mechanical renames and look correct. A few observations:

Potential Issue: Redundant alterationType check (SearchService.java:341)

} else if (StringUtils.equals(category, Category.ALLELE.getName()) && StringUtils.equals((String) result.get("alterationType"), "allele")) {

Now that alleles and variants have separate categories (allele_search_result vs variant_search_result), the alterationType == "allele" guard is redundant — documents with category ALLELE should always be alleles. This isn't a bug, but it's dead logic that could be simplified.

Design Question: No related data links for standalone Variant results

The Gene category now links to both ALLELE and VARIANT (correct split). However, Disease and Model only link to ALLELE, not VARIANT. And there's no else if (category == VARIANT) branch to give variant results their own related data links. If variants should surface related genes/diseases/models, this would need to be added. If variants intentionally don't have these relationships, this is fine as-is.

Verified: Field assignments in indexer removal is safe

The removed setAlterationType("variant"), setAlterationTypeSortOrder(4), setHasPhenotype(false), setHasDisease(false) from VariantSummaryCurationIndexer are already set in VariantSummaryConverter.java:264-268, so no data loss.

Verified: Null check removal is consistent

The removed null/empty checks in VariantSummaryCurationIndexer match the existing pattern in AlleleSummaryCurationIndexer, which also calls indexDocuments(response.getResults()) directly.

Overall the changes look correct and internally consistent.

@oblodgett oblodgett merged commit d9c4fdf into stage Apr 1, 2026
5 checks passed
@oblodgett oblodgett deleted the SCRUM-5953 branch April 1, 2026 17:33
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.

1 participant