SACGF / SACGF/variantgrid

OMIM built-in filter: drop the Gene join from the node count query

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

Description

🤖 Written by Claude.

Follow-up to #1722 — the OMIM built-in filter was the one item #1726 left out ("needs measurement rather than a blind rewrite").

### The filter today

`analysis/models/nodes/node_counts.py` (`get_extra_filters_q`):

```python
elif extra_filters == BuiltInFilters.OMIM:
q = Q(variantannotation__gene__geneannotation__omim_terms__isnull=False)
```

It runs on every node count (aggregated per analysis node), on the `BuiltInFilterNode` queryset, on the node grid when a count label is clicked, and for node column summaries.

### The version claim in #1722 is wrong

#1722 said this matches rows from every `GeneAnnotationVersion` because there's no `version=` filter. It doesn't. Every caller applies the Q to a queryset from `get_variant_queryset_for_annotation_version`, which attaches `AnnotationVersion.sql_partition_transformer` — that rewrites `"annotation_geneannotation"` in the generated SQL to the version's inherits-partition (`annotation_geneannotation_version_2`), the same way it does for `annotation_variantannotation`. Confirmed by dumping the SQL of a node-count aggregate on a dev DB (17M variants):

```
['"annotation_geneannotation_version_2"', '"annotation_variantannotation_version_2"', ...]
```

So there are no cross-version counts to fix, and the fix below does not change any counts.

### What is worth fixing

The join itself. `variantannotation__gene__geneannotation` walks `VariantAnnotation → Gene → GeneAnnotation`, and the `genes_gene` hop carries no filter — it exists only to get from `variantannotation.gene_id` to `geneannotation.gene_id`, which are the same value. `EXPLAIN (ANALYZE, BUFFERS)` on a whole-build count:

```
-> Hash Left Join Hash Cond: (genes_gene.identifier = annotation_geneannotation_version_2.gene_id)
-> Parallel Hash Left Join Hash Cond: (annotation_variantannotation_version_2.gene_id = genes_gene.identifier)
...
-> Parallel Seq Scan on genes_gene (rows=72782, buffers: shared read=5399)
```

A full seq scan + hash build of `genes_gene` (~73k rows, ~42MB of buffers) per count, purely to bridge two columns that already match.

Resolving the gene ids from `GeneAnnotation` as a subquery drops both joins:

```python
gene_annotation_qs = GeneAnnotation.objects.filter(version=gene_annotation_version, omim_terms__isnull=False)
q = Q(variantannotation__gene__in=gene_annotation_qs.values_list("gene_id", flat=True))
```

The subquery hits `annotation_geneannotation_..._gene_id_idx`-backed lookups against a small table (26,390 rows in the version partition, 4,141 with OMIM terms) and is planned once as a hashed SubPlan, not per row.

Filtering `version=` explicitly in Python also means the OMIM filter no longer relies on SQL string rewriting for its version scoping — it stays correct if the Q is ever applied to a queryset without the transformer hook, or on a deployment with `ANNOTATION_GENE_ANNOTATION_VERSION_ENABLED = False` (Shariant).

### Measurements (dev DB, 17M variants, GRCh38, warm cache)

Counting the OMIM filter over a window of variants, two runs each:

| variants scanned | current 3-join | subquery |
| --- | --- | --- |
| 1M | 1.28s / 1.27s | 1.27s / 1.28s |
| 5M | 1.55s / 1.55s | 1.51s / 1.50s |
| 17M (whole build) | 2.49s / 2.46s | 2.19s / 2.19s |

Cold cache the gap is wider (first-ever run of the 1M window: 3.22s vs 1.27s), because the current form has to read `genes_gene` off disk.

This is a modest win, not a #1720-sized one — the honest summary is that it removes work the query never needed, and is most visible when `genes_gene` isn't already cached. Counts are identical: every node of every analysis on the dev DB was counted both ways, 0 mismatches.

### Scope

- `get_extra_filters_q` takes the `AnnotationVersion` instead of the `GenomeBuild` (it needs the `GeneAnnotationVersion`; `genome_build` comes off the annotation version for the classification filters). Callers all have `analysis.annotation_version` in hand.
- Test coverage for the OMIM built-in filter, which currently has none.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in analysis/models/nodes/node_counts.py at get_extra_filters_q, then inspect its callers and how each supplies annotation.version. Add coverage for the OMIM built-in filter and verify that counts remain unchanged; the change is done when the Gene join is removed, version scoping is explicit, and the tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
postgresql, python
Domain
backend, databases, performance, testing
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.