Liftover page - retry failed liftovers per tool #1273

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
 

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").
@@ -121,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
@@ -167,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
@@ -213,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 |

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.

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)
 
 

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,15 +260,17 @@ 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()
 
     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
 

snpdb/management/commands/liftover_alleles.py

@@ -1,15 +1,20 @@
 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
 
 
 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)

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:

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)