SACGF / SACGF/variantgrid

Incremental (since=) classification export scans the entire table — no-op Shariant sync takes ~50s

Open
#1,778 1 comment 0 reactions 1 assignee Assigned to @TheMadBug View on GitHub
Dominant language
Python
Stars
30
Forks
3
Avg merge
9h 28m
Merged PRs (30d)
42

Description

🤖 Written by Claude

A `shariant_download` sync with nothing to fetch takes ~51.5s. Nearly all of that time is server-side in `classification/api/classifications/export` — an incremental (`since=`) export does a **full-table scan** of classifications even when nothing has changed, because the `since` filter is applied in Python after every record has already been fetched and hydrated.

## What happens on a no-op incremental export

1. `ClassificationFilter.cms_qs` (`classification/views/exports/classification_export_filter.py` ~line 636) **never uses `since` in SQL**. It selects every last-published `ClassificationModification` (only share-level / exclude-lab filters in SQL), with ~11 `select_related` joins, ordered by `genomic_sort` — Postgres joins and sorts the whole table before returning the first row.
2. `_allele_data()` (~line 724) iterates the entire result via `.iterator(chunk_size=1000)`, hydrating a full ORM object per record — including the `published_evidence` JSONB blob — and runs `_record_issues()` / `get_c_hgvs()` on each.
3. Only then does `_passes_since()` (~line 600) check `cm.modified >= since` etc. per allele group, in Python, discarding essentially everything.

So a sync that finds nothing still pays for a full export of the classification table, minus JSON serialization.

## Proposed fix

Push the `since` test into SQL as a **pre-filter on allele ids**, preserving the existing "include the whole allele group if any member changed" semantics. `_passes_since` checks four conditions, each expressible as a cheap query:

- `ClassificationModification.modified >= since` (on `is_last_published=True` rows)
- `Classification.modified > since`
- classification ids from `_since_flagged_classification_ids` (already a single query over `FlagComment.created__gte`)
- `allele_info.latest_validation.modified > since`

Union these into a set of changed allele ids, then in `cms_qs` add `cms = cms.filter(classification__allele_info__allele_id__in=changed_allele_ids)` when `since` is set. Keep `_passes_since` in place as a correctness net — it becomes a no-op over a tiny set.

When nothing has changed, the main query matches zero rows and the request should drop from ~50s to well under a second; when a handful of records changed, only those alleles get hydrated.

## Supporting changes

- `ClassificationModification.modified` has no index (`created` does — `classification/models/classification.py` ~line 2277), and `Classification.modified` (from `TimeStampedModel`) is also unindexed. Add `db_index=True` to both so the no-op case is truly instant.
- The same win applies to every `since`-based consumer of this export (MVL / Alissa downloads use the same `ClassificationFilter`), not just the Shariant sync.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.