SACGF / SACGF/variantgrid

ImportedAlleleInfo stuck showing "liftover in-progress" hourglass after liftover has failed

Open
#1,696 0 comments 0 reactions 0 assignees View on GitHub
bug classification Liftover
Dominant language
Python
Stars
30
Forks
3
Avg merge
9h 28m
Merged PRs (30d)
42

Description

🤖 Written by Claude

## Summary

An `ImportedAlleleInfo` whose liftover has genuinely failed never reaches a terminal
status. It stays on `MATCHED_IMPORTED_BUILD`, which renders as an hourglass with the
tooltip "Variant liftover in-progress" — forever. Users read this as "still processing"
when in fact nothing further will happen.

Reported by @EmmaTudini on SACGF/variantgrid_private#2647, where it is the last
outstanding item ("this seems to happen for every variant that fails liftover in
Shariant prod at the moment"). Splitting it out so 2647 can close.

The only current remedy is the admin action **Mark as Completed**
(`classification/admin/classification_admin.py:1306`), which does by hand exactly what
should be happening automatically.

## Root cause

Two stacked faults in `classification/models/classification_variant_info_models.py`.

**1. `liftover_complete` is never passed.**

`ImportedAlleleInfo.relink_variants()` (`:1058`) calls:

```python
allele_info.refresh_and_save(force_update=force_update)
```

dropping the flag — even though its liftover caller
(`classification/signals/classification_liftover.py:22`) is invoked from
`liftover_run_complete_signal` and therefore knows the run has finished. No caller
anywhere in the codebase passes `liftover_complete=True`.

**2. Even if it were passed, it is immediately overwritten.**

`set_variant_and_save()` sets the status:

```python
if applied_all or (liftover_complete and applied_any): # :1006
self.status = ImportedAlleleInfoStatus.MATCHED_ALL_BUILDS
else:
self.status = ImportedAlleleInfoStatus.MATCHED_IMPORTED_BUILD

self.apply_validation()
self.update_status() # :1012 <-- discards the above
self.save()
```

`update_status()` (`:880`) recomputes purely from build presence:

```python
if self.grch37 and self.grch38:
self.status = ImportedAlleleInfoStatus.MATCHED_ALL_BUILDS
elif self.variant_info_for_imported_genome_build:
self.status = ImportedAlleleInfoStatus.MATCHED_IMPORTED_BUILD
else:
self.status = ImportedAlleleInfoStatus.FAILED
```

A record with the imported build matched and the other build missing always lands on
`MATCHED_IMPORTED_BUILD`, regardless of whether liftover is pending or exhausted.

The `liftover_complete` parameter has been dead since it was introduced: the
`update_status()` call landed in `70fbc4cda6` (2023-01-25), the parameter in `fe4a6c458`
(2023-02-27), and the two were never reconciled.

## Knock-on effects

- There is no status meaning "liftover attempted, all tools failed, this is final".
`FAILED` is reserved for *matching* failure, and these records did match — they have a
usable variant for the imported build.
- Automatic status and admin cleanup disagree. `mark_as_completed` uses "allele set and
*either* build present → `MATCHED_ALL_BUILDS`"; `update_status()` requires *both*.
- The Slack health check
(`classification/signals/classification_health_checks.py:50`) counts these under
":hourglass_flowing_sand: Imported Allele Matching in status of ...", so stuck records
permanently inflate that figure.

## Proposed fix

The information needed is already recorded — `AlleleLiftover` distinguishes a run still
going from one that has failed:

- `AlleleLiftover.get_unfinished_liftover_run(allele, genome_build)` — `snpdb/models/models_variant.py:1148`
- `AlleleLiftover.get_last_failed_liftover_run(allele, genome_build)` — `snpdb/models/models_variant.py:1138`

1. Thread `liftover_complete=True` from `relink_variants(liftover_run=...)` through
`refresh_and_save()` to `set_variant_and_save()`.
2. Make `update_status()` the single place status is decided, and have it treat a
missing build as terminal when liftover is complete (or when the allele has a failed
liftover and no unfinished run) rather than as "in progress".
3. Settle the terminal state. Recommended: `MATCHED_ALL_BUILDS`, meaning "we have
finished trying", with the missing build surfaced through the existing
`missing_37` / `missing_38` validation tags that already carry severity "W". That
aligns automatic behaviour with what `mark_as_completed` already does by hand, and
keeps `FAILED` meaning matching failure. Alternative, if the distinction should be
visible in its own right: add a separate status/icon for "liftover unavailable".
4. Add a `ManualOperation` migration (or management command) to re-run status
calculation over the existing backlog — Shariant prod has accumulated these since at
least 2024.

## Testing

- Import a classification whose variant cannot be lifted over to the other build; after
the liftover run completes the allele info should show a terminal state, not an
hourglass.
- Import one that lifts over normally — should be unaffected.
- Confirm the existing stuck records in Shariant prod clear after the upgrade task runs.
- Confirm the health check hourglass count drops to genuinely in-flight records.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reading ImportedAlleleInfo.update_status(), set_variant_and_save(), relink_variants(), and refresh_and_save() in classification/models/classification_variant_info_models.py, then trace classification/signals/classification_liftover.py and the AlleleLiftover helpers in snpdb/models/models_variant.py. Exercise failed and successful liftover cases described in the issue. Done means failed liftover records reach a terminal state, successful liftover is unchanged, existing stuck records can be recalculated, and the health-check hourglass count reflects only in-flight work.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.