SACGF / SACGF/variantgrid

Discarded queryset filter and dead queries on hot page paths

Open
#1,724 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.

Three places where a query runs but its result is thrown away. Found while sweeping for optimisation wins after #1720. Results stay correct in each case, so these are wasted work rather than wrong output — but all three sit on hot page paths.

### 1. Discarded queryset filter scans every open pending-changes flag

`classification/models/classification_groups.py:136`

```python
flags_qs = Flag.objects.filter(
flag_type=classification_flag_types.classification_pending_changes,
resolution__status=FlagStatus.OPEN)

if self._modifications:
flag_collections_ids = {mod.classification.flag_collection_id for mod in self._modifications}
flags_qs.filter(collection_id__in=flag_collections_ids) # result discarded
```

Querysets are immutable, so the narrowing filter is a no-op and the loop below iterates every open pending-changes flag in the database instead of the handful belonging to these classifications. The resulting map is keyed by `collection_id` and only ever read for the modifications in hand, so output is unaffected — but the work grows with total database size.

Reached from the `{% classification_groups %}` template tag (`classification/templatetags/classification_tags.py:120`), so it runs on allele pages, gene symbol classification pages, variant details pages and discordance reports.

### 2. Dead `FlagComment` query in the flags payload

`flags/views/views.py:254`

```python
if flag_type.only_one:
# find when flag was last opened
try:
created = FlagComment.objects.filter(flag=flag, resolution__status=FlagStatus.OPEN).order_by('-created').\
values_list('created', flat=True).first()
except Exception:
pass
```

`created` is never read — the payload built immediately below uses `flag.created` (line 271). One query per qualifying flag, on an endpoint that fires on every classification, allele, variant and discordance page.

### 3. Unreachable branch materialises whole querysets

`uicore/templatetags/english_tags.py:11`

```python
if hasattr(items, '__len__'):
item_count = len(items)
elif isinstance(items, QuerySet):
item_count = items.count()
```

`QuerySet` defines `__len__`, so the `isinstance` branch can never be reached and `{% count qs ... %}` always evaluates the full queryset to produce a number. Current callers pass small collections so impact today is low, but it is a trap for the next caller.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with classification/models/classification_groups.py:136, then inspect flags/views/views.py:254 and uicore/templatetags/english_tags.py:11 along with their page and template-tag callers. Confirm each query result is either used or removed, and that queryset counting avoids materialising more data than needed while preserving the existing output on the listed page paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, databases, performance
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.