From a250f3d92d82b22c7e3bdb813eb1a28d43adf7a5 Mon Sep 17 00:00:00 2001 From: Dave Lawrence Date: Tue, 8 Sep 2026 12:33:13 +0000 Subject: [PATCH 1/2] Liftover page - retry failed liftovers per tool #1273 A tool that has ever errored on an allele/build was skipped forever by AlleleLiftover.get_failed_conversion_tools, so the liftover page's "Liftover variants" button, the allele page's "Create Variant" button and the admin action all silently did nothing for previously-failed alleles. Add retry_conversion_tools, subtracted from the per-allele failed set in _get_build_liftover_dicts. Earlier AlleleLiftover rows are kept as history - a retry that fails again lands as a new row under a new LiftoverRun. - Liftover page: a per-tool "Retry failed liftovers" button under each "Failed Liftover to " heading, with the count of alleles it would re-attempt (Allele.failed_liftover_for_build) - liftover_alleles / liftover_allele_batch tasks take the tool value - "Create Variant" and the admin Liftover action retry every tool, so the button also appears for an allele all tools have failed on - manage.py liftover_alleles --retry-tool --- classification/variant_card.py | 5 +- .../plans/1273_liftover_retry_failed_plan.md | 2 + snpdb/CLAUDE.md | 3 + snpdb/admin.py | 5 +- snpdb/liftover.py | 46 +++++++++---- snpdb/management/commands/liftover_alleles.py | 11 ++- snpdb/models/models_variant.py | 9 +++ snpdb/tasks/liftover_tasks.py | 28 +++++--- .../snpdb/liftover/liftover_runs.html | 27 ++++++++ snpdb/tests/test_liftover.py | 69 +++++++++++++++++++ snpdb/views/views_liftover.py | 47 ++++++++++--- .../default_templates/changelog.html | 10 +++ variantopedia/views_allele.py | 4 +- 13 files changed, 229 insertions(+), 37 deletions(-) diff --git a/classification/variant_card.py b/classification/variant_card.py index 3b1780f86..c57b83409 100644 --- a/classification/variant_card.py +++ b/classification/variant_card.py @@ -11,6 +11,7 @@ from snpdb.liftover import allele_can_attempt_liftover from snpdb.models import ( Allele, + AlleleConversionTool, AlleleLiftover, AlleleMergeLog, AlleleOrigin, @@ -40,7 +41,9 @@ def __init__(self, user: User, allele: Allele, genome_build: GenomeBuild): if unfinished_liftover is None: try: check_can_create_variants(user) - can_create_variant = allele_can_attempt_liftover(allele, genome_build) + # An explicit ask by a user, so offer tools that have already failed on this allele + can_create_variant = allele_can_attempt_liftover(allele, genome_build, + retry_conversion_tools=list(AlleleConversionTool)) except CreateManualVariantForbidden: pass diff --git a/claude/plans/1273_liftover_retry_failed_plan.md b/claude/plans/1273_liftover_retry_failed_plan.md index de1111dcd..a62a15fd1 100644 --- a/claude/plans/1273_liftover_retry_failed_plan.md +++ b/claude/plans/1273_liftover_retry_failed_plan.md @@ -2,6 +2,8 @@ Written by Claude Fable 5 (claude-fable-5), 2026-08-31 +Status: in progress + [#1273](https://github.com/SACGF/variantgrid/issues/1273): after fixing a bug or config problem we want to re-run liftover for the alleles that failed, broken down by tool (e.g. "relaunch all failed bcftools +liftover jobs"). diff --git a/snpdb/CLAUDE.md b/snpdb/CLAUDE.md index 143607340..65113433c 100644 --- a/snpdb/CLAUDE.md +++ b/snpdb/CLAUDE.md @@ -38,6 +38,9 @@ Gotchas: - models/models_vcf.py:VCF.delete_internal_data keeps the VCF and Sample rows and drops or recreates the partitions (recreate_partitions=False is the archive path in archive.py). - Whole-table work on snpdb_variant or snpdb_allele is millions of rows in prod: page by pk range and fan out celery tasks (tasks/liftover_tasks.py:liftover_allele_batch, settings.LIFTOVER_BATCH_SIZE) rather than iterating one queryset. - Liftover is per Allele, not per Variant: liftover.py:create_liftover_pipelines batches AlleleLiftover records and liftover.py:allele_can_attempt_liftover decides eligibility. Builds sharing a contig link with AlleleConversionTool.SAME_CONTIG and no external call. +- A tool that has ever errored on an allele/build is skipped forever (models/models_variant.py:AlleleLiftover.get_failed_conversion_tools), + which is why re-clicking liftover does nothing. Pass `retry_conversion_tools=` to liftover.py:create_liftover_pipelines to + override it for chosen tools - the liftover page's per-tool retry buttons, "Create Variant" and the admin action all do (#1273). - ClinGen Allele Registry calls are network I/O (clingen_allele.py:populate_clingen_alleles_for_variants); models/models_variant.py:Variant.can_have_clingen_allele bounds what may be sent. - Somalier decides how to genotype from the header of the VCF we hand it: a FORMAT `AD` line means it re-genotypes every sample from the depths and applies its own QC at relate time (`--min-depth` 7, `--min-ab` 0.3), no `AD` line means it trusts `GT`. variants_to_vcf.py:vcf_export_to_file picks one per VCF - declaring `AD` we can't fill in zeroes out every sample (#183). - somalier keeps each site's two alleles alphabetically and reads the genotype against that pair rather than against the record's `REF`/`ALT` (brentp/somalier#163), so variants_to_vcf.py:vcf_export_to_file writes the `AD` pair in the site's order - `REF`, `ALT` and `GT` stay as called, and only a VCF with no depths flips `GT` instead. Getting it wrong is invisible in a jointly called VCF - it cancels out pairwise - and shows up as inflated relatedness once `--unknown` is in play (#183). `settings.SOMALIER["compensate_allele_order"]` turns it off for a somalier that reads the record's alleles; `deployment_check`'s `somalier_allele_order` genotypes both allele orders through the installed binary and fails saying which way to set it, so nobody has to remember (variantgrid/deployment_validation/somalier_check.py). Changing it means rebuilding the extracts. diff --git a/snpdb/admin.py b/snpdb/admin.py index 2c6828d4d..1d12202c8 100644 --- a/snpdb/admin.py +++ b/snpdb/admin.py @@ -18,6 +18,7 @@ from snpdb.liftover import liftover_alleles from snpdb.models import ( Allele, + AlleleConversionTool, AlleleLiftover, ClinVarKey, ClinVarKeyExcludePattern, @@ -86,7 +87,9 @@ def variants(self, obj: Allele): @admin_action("Liftover") def liftover(self, request, queryset): - liftover_alleles(allele_qs=queryset, user=request.user) + # Explicitly selected alleles, so retry tools that have already failed on them + liftover_alleles(allele_qs=queryset, user=request.user, + retry_conversion_tools=list(AlleleConversionTool)) self.message_user(request, message='Liftover queued', level=messages.INFO) diff --git a/snpdb/liftover.py b/snpdb/liftover.py index c231202aa..ef1479120 100644 --- a/snpdb/liftover.py +++ b/snpdb/liftover.py @@ -43,16 +43,21 @@ def create_liftover_pipelines(user: User, alleles: Iterable[Allele], import_source: ImportSource, inserted_genome_build: GenomeBuild, - destination_genome_builds: list[GenomeBuild] = None): + destination_genome_builds: list[GenomeBuild] = None, + retry_conversion_tools: Iterable[AlleleConversionTool] = ()): """ Creates and runs a liftover pipeline for each destination GenomeBuild (default = all other builds) Alleles are handled in batches of settings.LIFTOVER_BATCH_SIZE - a batch's alleles, coordinates and AlleleLiftover records are all held in memory while its VCF is written, and anything that goes wrong - only takes out the batch rather than the whole run """ + only takes out the batch rather than the whole run + + retry_conversion_tools are attempted even for alleles they have already failed on (@see + _get_build_liftover_dicts) - use after fixing a bug or config problem that caused the failures """ for allele_batch in _batch_alleles(alleles): _create_liftover_pipelines_for_batch(user, allele_batch, import_source, - inserted_genome_build, destination_genome_builds) + inserted_genome_build, destination_genome_builds, + retry_conversion_tools=retry_conversion_tools) def _batch_alleles(alleles: Iterable[Allele]) -> Iterable[list[Allele]]: @@ -74,8 +79,9 @@ def _batch_alleles(alleles: Iterable[Allele]) -> Iterable[list[Allele]]: def _create_liftover_pipelines_for_batch(user: User, alleles: list[Allele], import_source: ImportSource, inserted_genome_build: GenomeBuild, - destination_genome_builds: list[GenomeBuild] = None): - build_liftover_existing_allele_and_variants, build_liftover_allele_variant_coordinate_error = _get_build_liftover_dicts(alleles, inserted_genome_build, destination_genome_builds) + destination_genome_builds: list[GenomeBuild] = None, + retry_conversion_tools: Iterable[AlleleConversionTool] = ()): + build_liftover_existing_allele_and_variants, build_liftover_allele_variant_coordinate_error = _get_build_liftover_dicts(alleles, inserted_genome_build, destination_genome_builds, retry_conversion_tools=retry_conversion_tools) for genome_build, liftover_tuples in build_liftover_existing_allele_and_variants.items(): for conversion_tool, av_tuples in liftover_tuples.items(): liftover = LiftoverRun.objects.create(user=user, @@ -190,8 +196,12 @@ def _variant_allele_for_build(allele, genome_build: GenomeBuild) -> Optional['Va def _get_build_liftover_dicts(alleles: Iterable[Allele], inserted_genome_build: GenomeBuild, - destination_genome_builds: list[GenomeBuild] = None) -> tuple[dict, dict]: - """ ID column set to allele_id """ + destination_genome_builds: list[GenomeBuild] = None, + retry_conversion_tools: Iterable[AlleleConversionTool] = ()) -> tuple[dict, dict]: + """ ID column set to allele_id + + Tools in retry_conversion_tools are dropped from each allele's already-failed set, so they are + attempted again - the earlier AlleleLiftover records are kept as history """ if destination_genome_builds is None: destination_genome_builds = GenomeBuild.builds_with_annotation() @@ -208,6 +218,7 @@ def _get_build_liftover_dicts(alleles: Iterable[Allele], inserted_genome_build: allele_ids = [allele.pk for allele in alleles] alleles = _liftover_allele_qs(allele_ids) build_failed_tools = {gb: AlleleLiftover.get_failed_conversion_tools(allele_ids, gb) for gb in other_builds} + retry_tools = set(retry_conversion_tools) build_liftover_existing_allele_and_variants = defaultdict(lambda: defaultdict(list)) # Already lifted over build_liftover_allele_variant_coordinate_error = defaultdict(lambda: defaultdict(list)) # Need to run pipelines @@ -230,7 +241,7 @@ def _get_build_liftover_dicts(alleles: Iterable[Allele], inserted_genome_build: continue hgvs_matcher = HGVSMatcher.instance(genome_build) - failed_tools = build_failed_tools[genome_build].get(allele.pk, set()) + failed_tools = build_failed_tools[genome_build].get(allele.pk, set()) - retry_tools for tool_coordinate_error in itertools.chain( _liftover_using_dest_variant_coordinate(allele, genome_build, @@ -249,7 +260,8 @@ def _get_build_liftover_dicts(alleles: Iterable[Allele], inserted_genome_build: return build_liftover_existing_allele_and_variants, build_liftover_allele_variant_coordinate_error -def liftover_alleles(allele_qs, user: User = None): +def liftover_alleles(allele_qs, user: User = None, + retry_conversion_tools: Iterable[AlleleConversionTool] = ()): """ Creates then runs (async) liftover pipelines for a queryset of alleles """ if user is None: user = admin_bot() @@ -257,7 +269,8 @@ def liftover_alleles(allele_qs, user: User = None): for genome_build in GenomeBuild.builds_with_annotation(): variants_qs = Variant.objects.filter(variantallele__allele__in=allele_qs) populate_clingen_alleles_for_variants(genome_build, variants_qs) - create_liftover_pipelines(user, allele_qs, ImportSource.WEB, inserted_genome_build=genome_build) + create_liftover_pipelines(user, allele_qs, ImportSource.WEB, inserted_genome_build=genome_build, + retry_conversion_tools=retry_conversion_tools) def _run_liftover_using_same_contig(liftover, av_tuples: list[tuple[Allele, Variant]]): @@ -428,19 +441,26 @@ def _liftover_using_source_variant_coordinate(allele, source_genome_build: Genom break # Just want 1st one -def allele_can_attempt_liftover(allele, genome_build) -> bool: +def allele_can_attempt_liftover(allele, genome_build, + retry_conversion_tools: Iterable[AlleleConversionTool] = ()) -> bool: + """ retry_conversion_tools are considered available even if they have already failed on this allele """ conversion_tool, variant = _liftover_using_existing_contig(allele, genome_build) if conversion_tool and variant: return True - for conversion_tool, variant_coordinate, _error_message in _liftover_using_dest_variant_coordinate(allele, genome_build): + failed_tools = AlleleLiftover.get_failed_conversion_tools([allele.pk], genome_build).get(allele.pk, set()) + failed_tools -= set(retry_conversion_tools) + + for conversion_tool, variant_coordinate, _error_message in _liftover_using_dest_variant_coordinate( + allele, genome_build, failed_tools=failed_tools): if conversion_tool and variant_coordinate: return True for va in allele.variantallele_set.all(): if va.genome_build_id == genome_build.pk: continue - for conversion_tool, variant_coordinate, _error_message in _liftover_using_source_variant_coordinate(allele, va.genome_build, genome_build): + for conversion_tool, variant_coordinate, _error_message in _liftover_using_source_variant_coordinate( + allele, va.genome_build, genome_build, failed_tools=failed_tools): if conversion_tool and variant_coordinate: return True diff --git a/snpdb/management/commands/liftover_alleles.py b/snpdb/management/commands/liftover_alleles.py index f34920210..6bf2fc3eb 100644 --- a/snpdb/management/commands/liftover_alleles.py +++ b/snpdb/management/commands/liftover_alleles.py @@ -1,7 +1,7 @@ from django.core.management.base import BaseCommand from library.guardian_utils import admin_bot -from snpdb.models import GenomeBuild +from snpdb.models import AlleleConversionTool, GenomeBuild from snpdb.tasks.liftover_tasks import liftover_alleles @@ -9,7 +9,12 @@ class Command(BaseCommand): category = "ops" help = "Lifts over any alleles not in both genome builds" - def handle(self, **options): + def add_arguments(self, parser): + parser.add_argument('--retry-tool', choices=[act.value for act in AlleleConversionTool], + help="Only re-attempt this conversion tool, for the alleles it has already failed on") + + def handle(self, *args, **options): user = admin_bot() + retry_tool = options["retry_tool"] for genome_build in GenomeBuild.builds_with_annotation(): - liftover_alleles(user.username, genome_build.name) + liftover_alleles(user.username, genome_build.name, retry_tool) diff --git a/snpdb/models/models_variant.py b/snpdb/models/models_variant.py index 6d10ad372..08de4c7b9 100644 --- a/snpdb/models/models_variant.py +++ b/snpdb/models/models_variant.py @@ -243,6 +243,15 @@ def missing_variants_for_build(genome_build) -> QuerySet['Allele']: # distinct as the variantallele join returns an allele once per build it's already in return alleles_with_variants_qs.filter(~Q(variantallele__genome_build=genome_build)).distinct() + @staticmethod + def failed_liftover_for_build(genome_build, conversion_tool) -> QuerySet['Allele']: + """ Alleles still missing a variant in genome_build, where conversion_tool has already failed on them. + Mirrors AlleleLiftover.get_failed_conversion_tools - ie exactly the alleles a retry re-attempts """ + return Allele.missing_variants_for_build(genome_build).filter( + alleleliftover__status=ProcessingStatus.ERROR, + alleleliftover__liftover__genome_build=genome_build, + alleleliftover__liftover__conversion_tool=conversion_tool).distinct() + def __str__(self): name = f"Allele {self.pk}" if self.clingen_allele: diff --git a/snpdb/tasks/liftover_tasks.py b/snpdb/tasks/liftover_tasks.py index 34112e454..30a346129 100644 --- a/snpdb/tasks/liftover_tasks.py +++ b/snpdb/tasks/liftover_tasks.py @@ -12,21 +12,30 @@ @celery.shared_task -def liftover_alleles(username, genome_build_name): - """ Queues a task per batch of alleles - the db_workers pool caps how many run at once """ +def liftover_alleles(username, genome_build_name, retry_conversion_tool: str = None): + """ Queues a task per batch of alleles - the db_workers pool caps how many run at once + + retry_conversion_tool (an AlleleConversionTool value) restricts this to the alleles that tool has + failed on, and re-attempts it for them - @see create_liftover_pipelines """ genome_build = GenomeBuild.get_name_or_alias(genome_build_name) - allele_qs = Allele.missing_variants_for_build(genome_build) + allele_qs = _alleles_to_liftover(genome_build, retry_conversion_tool) for other_build in GenomeBuild.builds_with_annotation(): if not GenomeBuild.is_equivalent(genome_build, other_build): num_batches = 0 for min_allele_id, max_allele_id in _allele_id_batches(allele_qs): liftover_allele_batch.si(username, genome_build.name, other_build.name, - min_allele_id, max_allele_id).apply_async() + min_allele_id, max_allele_id, retry_conversion_tool).apply_async() num_batches += 1 logging.info("Queued %d liftover batches from %s to %s", num_batches, other_build, genome_build) +def _alleles_to_liftover(genome_build: GenomeBuild, retry_conversion_tool: str = None) -> QuerySet[Allele]: + if retry_conversion_tool: + return Allele.failed_liftover_for_build(genome_build, retry_conversion_tool) + return Allele.missing_variants_for_build(genome_build) + + def _allele_id_batches(allele_qs: QuerySet) -> Iterable[tuple[int, int]]: """ Inclusive (min, max) allele id ranges, each covering LIFTOVER_BATCH_SIZE alleles """ batch_size = settings.LIFTOVER_BATCH_SIZE @@ -36,13 +45,16 @@ def _allele_id_batches(allele_qs: QuerySet) -> Iterable[tuple[int, int]]: @celery.shared_task -def liftover_allele_batch(username, genome_build_name, other_build_name, min_allele_id, max_allele_id): +def liftover_allele_batch(username, genome_build_name, other_build_name, min_allele_id, max_allele_id, + retry_conversion_tool: str = None): user = User.objects.get(username=username) genome_build = GenomeBuild.get_name_or_alias(genome_build_name) other_build = GenomeBuild.get_name_or_alias(other_build_name) # Re-query, so any alleles lifted over since the batches were worked out drop out - alleles = Allele.missing_variants_for_build(genome_build).filter(pk__gte=min_allele_id, - pk__lte=max_allele_id) + alleles = _alleles_to_liftover(genome_build, retry_conversion_tool).filter(pk__gte=min_allele_id, + pk__lte=max_allele_id) + retry_conversion_tools = [retry_conversion_tool] if retry_conversion_tool else [] logging.info("creating liftover pipelines from %s to %s for alleles %d-%d", other_build, genome_build, min_allele_id, max_allele_id) - create_liftover_pipelines(user, alleles, ImportSource.WEB, other_build, [genome_build]) + create_liftover_pipelines(user, alleles, ImportSource.WEB, other_build, [genome_build], + retry_conversion_tools=retry_conversion_tools) diff --git a/snpdb/templates/snpdb/liftover/liftover_runs.html b/snpdb/templates/snpdb/liftover/liftover_runs.html index 80e11c690..a3d5164d5 100644 --- a/snpdb/templates/snpdb/liftover/liftover_runs.html +++ b/snpdb/templates/snpdb/liftover/liftover_runs.html @@ -82,6 +82,33 @@

Liftover Runs

Failed Liftover to {{ genome_build }}

+ {% with tool_counts=retry_counts|get_item:genome_build.name %} + {% if tool_counts %} +

A retry re-attempts that tool for every allele it has failed on that still + has no {{ genome_build }} variant. Earlier attempts are kept, and shown below.

+
+ {% csrf_token %} + + + + + + + + + + {% for tool, count in tool_counts %} + + + + + + {% endfor %} + +
ToolAlleles Still MissingAction
{{ tool.label }}{{ count|intcomma }}
+
+ {% endif %} + {% endwith %}
{% endfor %} diff --git a/snpdb/tests/test_liftover.py b/snpdb/tests/test_liftover.py index 2edc930fb..4382dd971 100644 --- a/snpdb/tests/test_liftover.py +++ b/snpdb/tests/test_liftover.py @@ -13,11 +13,13 @@ _liftover_using_source_variant_coordinate, _non_standard_contig_error, _run_liftover_using_same_contig, + allele_can_attempt_liftover, ) from snpdb.models import ( Allele, AlleleConversionTool, AlleleLiftover, + AlleleOrigin, GenomeBuild, LiftoverRun, ProcessingStatus, @@ -87,6 +89,44 @@ def _liftover_using_source_variant_coordinate(self): self.assertEqual(conversion_tool, AlleleConversionTool.BCFTOOLS_LIFTOVER) self.assertEqual(variant_coordinate_38, self.expected_vc_38) + def test_retry_conversion_tools_overrides_failed_set(self): + """ #1273 - a tool that has already failed on an allele is skipped forever unless explicitly retried """ + grch37 = GenomeBuild.grch37() + grch38 = GenomeBuild.grch38() + variant_37 = slowly_create_test_variant("3", 128198980, 'A', 'T', grch37) + VariantAllele.objects.create(variant=variant_37, genome_build=grch37, allele=self.allele, + origin=AlleleOrigin.IMPORTED_TO_DATABASE, + allele_linking_tool=AlleleConversionTool.CLINGEN_ALLELE_REGISTRY) + for conversion_tool in [AlleleConversionTool.CLINGEN_ALLELE_REGISTRY, + AlleleConversionTool.BCFTOOLS_LIFTOVER]: + liftover_run = LiftoverRun.objects.create(user=admin_bot(), conversion_tool=conversion_tool, + genome_build=grch38) + AlleleLiftover.objects.create(allele=self.allele, liftover=liftover_run, + status=ProcessingStatus.ERROR) + + _existing, needs_pipeline = _get_build_liftover_dicts([self.allele], grch37, [grch38]) + self.assertEqual(dict(needs_pipeline), {}, "Every tool failed, so nothing is attempted") + + retry_tools = {AlleleConversionTool.CLINGEN_ALLELE_REGISTRY} + _existing, needs_pipeline = _get_build_liftover_dicts([self.allele], grch37, [grch38], + retry_conversion_tools=retry_tools) + tools = needs_pipeline[grch38] + self.assertNotIn(AlleleConversionTool.BCFTOOLS_LIFTOVER, tools, "Tools not retried stay skipped") + _allele, variant_coordinate, _error = tools[AlleleConversionTool.CLINGEN_ALLELE_REGISTRY][0] + self.assertEqual(variant_coordinate, self.expected_vc_38) + + def test_allele_can_attempt_liftover_with_retry(self): + grch38 = GenomeBuild.grch38() + for conversion_tool in AlleleConversionTool: + liftover_run = LiftoverRun.objects.create(user=admin_bot(), conversion_tool=conversion_tool, + genome_build=grch38) + AlleleLiftover.objects.create(allele=self.allele, liftover=liftover_run, + status=ProcessingStatus.ERROR) + + self.assertFalse(allele_can_attempt_liftover(self.allele, grch38)) + self.assertTrue(allele_can_attempt_liftover(self.allele, grch38, + retry_conversion_tools=list(AlleleConversionTool))) + def test_standard_contig_written_to_vcf(self): self.assertIsNone(_non_standard_contig_error(GenomeBuild.grch37(), self.expected_vc_37)) @@ -128,6 +168,35 @@ def test_allele_id_batches(self): self.assertEqual(list(_allele_id_batches(allele_qs)), expected) +class TestFailedLiftoverAlleles(TestCase): + """ #1273 - which alleles a per-tool retry re-attempts """ + + @classmethod + def setUpTestData(cls): + grch37 = GenomeBuild.grch37() + grch38 = GenomeBuild.grch38() + cls.still_missing = create_mock_allele(slowly_create_test_variant("3", 1000, 'A', 'T', grch37), grch37) + cls.lifted_over = create_mock_allele(slowly_create_test_variant("3", 2000, 'A', 'T', grch37), grch37) + VariantAllele.objects.create(variant=slowly_create_test_variant("3", 2001, 'A', 'T', grch38), + genome_build=grch38, allele=cls.lifted_over, + origin=AlleleOrigin.LIFTOVER, + allele_linking_tool=AlleleConversionTool.BCFTOOLS_LIFTOVER) + + liftover_run = LiftoverRun.objects.create(user=admin_bot(), + conversion_tool=AlleleConversionTool.BCFTOOLS_LIFTOVER, + genome_build=grch38) + for allele in [cls.still_missing, cls.lifted_over]: + AlleleLiftover.objects.create(allele=allele, liftover=liftover_run, status=ProcessingStatus.ERROR) + + def test_failed_liftover_for_build(self): + grch38 = GenomeBuild.grch38() + qs = Allele.failed_liftover_for_build(grch38, AlleleConversionTool.BCFTOOLS_LIFTOVER) + self.assertEqual(list(qs), [self.still_missing], "An allele that has since been lifted over is left alone") + + qs = Allele.failed_liftover_for_build(grch38, AlleleConversionTool.CLINGEN_ALLELE_REGISTRY) + self.assertEqual(list(qs), [], "Only the tool being retried counts") + + class TestLiftoverQueries(TestCase): @classmethod def setUpTestData(cls): diff --git a/snpdb/views/views_liftover.py b/snpdb/views/views_liftover.py index 513399f64..cc0cd65fa 100644 --- a/snpdb/views/views_liftover.py +++ b/snpdb/views/views_liftover.py @@ -1,5 +1,7 @@ +from typing import Optional from django.contrib import messages +from django.db.models import Count from django.shortcuts import render from library.django_utils import ( @@ -23,19 +25,23 @@ def liftover_runs(request): genome_builds = GenomeBuild.builds_with_annotation() if request.method == 'POST': - dest_genome_build = None - for genome_build in genome_builds: - if f"liftover_to_{genome_build.name}" in request.POST: - dest_genome_build = genome_build - break - if dest_genome_build is None: - raise ValueError("Could not determine dest genome build from liftover_runs POST") - messages.add_message(request, messages.INFO, f"Lifting over alleles to {dest_genome_build}") - liftover_alleles.si(request.user.username, dest_genome_build.name).apply_async() + dest_genome_build, retry_conversion_tool = _liftover_post_action(request.POST, genome_builds) + if retry_conversion_tool: + num_alleles = Allele.failed_liftover_for_build(dest_genome_build, retry_conversion_tool).count() + message = f"Retrying {retry_conversion_tool.label} liftover to {dest_genome_build} " \ + f"for {num_alleles} alleles" + else: + message = f"Lifting over alleles to {dest_genome_build}" + messages.add_message(request, messages.INFO, message) + liftover_alleles.si(request.user.username, dest_genome_build.name, + retry_conversion_tool.value if retry_conversion_tool else None).apply_async() alleles_missing_variants = {} + retry_counts = {} for genome_build in genome_builds: - alleles_missing_variants[genome_build.name] = Allele.missing_variants_for_build(genome_build).count() + missing_qs = Allele.missing_variants_for_build(genome_build) + alleles_missing_variants[genome_build.name] = missing_qs.count() + retry_counts[genome_build.name] = _failed_tool_counts(genome_build, missing_qs) qs_allele_liftover = AlleleLiftover.objects.all() @@ -59,6 +65,7 @@ def liftover_runs(request): context = { "genome_builds": genome_builds, "alleles_missing_variants": alleles_missing_variants, + "retry_counts": retry_counts, "processing_status_cols": processing_status_cols, "tool_status": tool_status, "failure_rate": failure_rate, @@ -67,6 +74,26 @@ def liftover_runs(request): return render(request, "snpdb/liftover/liftover_runs.html", context) +def _liftover_post_action(post, genome_builds) -> tuple[GenomeBuild, Optional[AlleleConversionTool]]: + """ Buttons are named liftover_to_{build} (everything missing) or retry_{build}_{tool} (that tool's failures) """ + for genome_build in genome_builds: + if f"liftover_to_{genome_build.name}" in post: + return genome_build, None + for act in AlleleConversionTool: + if f"retry_{genome_build.name}_{act.value}" in post: + return genome_build, act + raise ValueError("Could not determine dest genome build from liftover_runs POST") + + +def _failed_tool_counts(genome_build, missing_qs) -> list[tuple[AlleleConversionTool, int]]: + """ (tool, number of alleles a retry would re-attempt) for each tool with failures, in enum order """ + qs = AlleleLiftover.objects.filter(liftover__genome_build=genome_build, + status=ProcessingStatus.ERROR, + allele__in=missing_qs) + counts = dict(qs.values_list("liftover__conversion_tool").annotate(num_alleles=Count("allele", distinct=True))) + return [(act, counts[act.value]) for act in AlleleConversionTool if counts.get(act.value)] + + @require_superuser def view_liftover_run(request, liftover_run_id): liftover_run = LiftoverRun.objects.get(pk=liftover_run_id) diff --git a/variantgrid/templates/default_templates/changelog.html b/variantgrid/templates/default_templates/changelog.html index 069d7c9c9..ce6082917 100644 --- a/variantgrid/templates/default_templates/changelog.html +++ b/variantgrid/templates/default_templates/changelog.html @@ -259,6 +259,16 @@

VCF upload

{% else %} +
+
8 September 2026
+
+

Liftover

+
    +
  • #1273 - Liftover page - retry failed liftovers per tool (eg all failed bcftools +liftover)
    • +
+
+
+
4 September 2026
diff --git a/variantopedia/views_allele.py b/variantopedia/views_allele.py index a0a1675d2..3d9ff4f75 100644 --- a/variantopedia/views_allele.py +++ b/variantopedia/views_allele.py @@ -181,5 +181,7 @@ def create_variant_for_allele(request, allele_id, genome_build_name): genome_build = get_genome_build_or_404(genome_build_name) non_liftover_origin = [AlleleOrigin.IMPORTED_TO_DATABASE, AlleleOrigin.IMPORTED_NORMALIZED] if variant_allele := allele.variantallele_set.filter(origin__in=non_liftover_origin).first(): - create_liftover_pipelines(admin_bot(), [allele], ImportSource.WEB, variant_allele.genome_build, [genome_build]) + # The user asked for this allele specifically, so retry every tool that has already failed on it + create_liftover_pipelines(admin_bot(), [allele], ImportSource.WEB, variant_allele.genome_build, [genome_build], + retry_conversion_tools=list(AlleleConversionTool)) return redirect(allele) From e07af33c3260dbee60c1c1f5c1bb302216f59601 Mon Sep 17 00:00:00 2001 From: Dave Lawrence Date: Tue, 8 Sep 2026 12:51:32 +0000 Subject: [PATCH 2/2] 1273 plan - cite the modules the liftover views actually live in --- claude/plans/1273_liftover_retry_failed_plan.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/claude/plans/1273_liftover_retry_failed_plan.md b/claude/plans/1273_liftover_retry_failed_plan.md index a62a15fd1..0f25c88cf 100644 --- a/claude/plans/1273_liftover_retry_failed_plan.md +++ b/claude/plans/1273_liftover_retry_failed_plan.md @@ -123,7 +123,7 @@ Everything else (batching by pk range, `log_traceback`, one task per batch) stay ## 4. Liftover page -### View — `snpdb/views/views.py:liftover_runs` +### View — `snpdb/views/views_liftover.py:liftover_runs` **POST**: alongside the existing `liftover_to_{build}` buttons, accept `retry_{build}_{tool}` where `tool` is the `AlleleConversionTool` value. Parse both in one loop over @@ -169,7 +169,7 @@ below. Bootstrap 4 (`btn btn-secondary`, `table`), matching the "Alleles Missing These are the two other places a human explicitly asks for a liftover, and both currently go quiet after every tool has failed. Same kwarg, so each is a one-liner: -- `variantopedia/views.py:create_variant_for_allele` — pass +- `variantopedia/views_allele.py:create_variant_for_allele` — pass `retry_conversion_tools=list(AlleleConversionTool)`: the user clicked "Create Variant", so try every tool again. - `classification/variant_card.py` / `allele_can_attempt_liftover()` — for the button to *appear* on an @@ -215,9 +215,9 @@ needed — the view logic is button-name parsing; the pipeline behaviour is cove | `snpdb/liftover.py` | `retry_conversion_tools` kwarg on `create_liftover_pipelines`, `_create_liftover_pipelines_for_batch`, `_get_build_liftover_dicts`, `liftover_alleles`, `allele_can_attempt_liftover` | | `snpdb/models/models_variant.py` | `Allele.failed_liftover_for_build()` | | `snpdb/tasks/liftover_tasks.py` | `retry_conversion_tool` on both tasks, `_alleles_to_liftover()` helper | -| `snpdb/views/views.py` | `liftover_runs`: parse `retry_{build}_{tool}` POST, `retry_counts` context | +| `snpdb/views/views_liftover.py` | `liftover_runs`: parse `retry_{build}_{tool}` POST, `retry_counts` context | | `snpdb/templates/snpdb/liftover/liftover_runs.html` | per-build retry table + note | -| `variantopedia/views.py` | `create_variant_for_allele` retries all tools | +| `variantopedia/views_allele.py` | `create_variant_for_allele` retries all tools | | `classification/variant_card.py` | `allele_can_attempt_liftover(..., retry_conversion_tools=list(AlleleConversionTool))` | | `snpdb/admin.py` | admin action retries all tools | | `snpdb/management/commands/liftover_alleles.py` | `--retry-tool` option |