Overlaps grid skew_status subquery returns an arbitrary next step, not the highest

Open Beginner friendly
  • Is something wrong?
#2,647 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
django, postgresql, python
Domain
backend, databases

Research direction

Read classification/views/overlaps_datatables.py and classification/views/overlaps_view.py to trace the five subquery call sites, then find the related overlap tests. Add a case with three next steps inserted from lowest to highest and check that the annotated skew_status is the highest; confirm the grid and download use the shared helper.

Written by the indexing model from the issue text.

Description

🤖 Written by Claude

The overlaps grid (classification/overlaps) and the overlaps CSV download annotate each overlap with skew_status. It's meant to be the highest triage next step among the contributions the lab picker selects. The subquery doesn't compute that.

OverlapContributionNextStep.objects.filter(lab_filter_q).filter(
    overlap=OuterRef('pk')
).annotate(max_status=Max('next_step')).values_list('max_status')[:1]

.annotate(Max(...)) with no .values() before it groups by the OverlapContributionNextStep row's own id. Each next step is then its own group, so Max is just that row's next_step, and [:1] with no ordering returns whichever row Postgres reads first. EXPLAIN shows GroupAggregate Group Key: "U0".id.

The same pattern is copied five times:

  • four times in OverlapColumns.get_initial_queryset (classification/views/overlaps_datatables.py);
  • once in download_overlaps (classification/views/overlaps_view.py).

Effect

  • Sort order: the grid's default sort (Summary column: overlap_status, skew_status, ...) and the download's ordering use an arbitrary lab's next step instead of the highest.
  • Tab filtering: "Awaiting Triage" and the other tabs filter next steps by next_step__in before the subquery, then only check that some match exists. Membership is right; only the value used for sorting is wrong.
  • Not affected:
    • the per-row "next step" label shown in the Summary cell, which uses a separate aggregate();
    • overlaps where every contribution has the same next step.

Fix

Order by next_step and take the first:

Subquery(next_steps_qs.order_by('-next_step').values('next_step')[:1])

Put it in one helper for the five call sites. A test can give an overlap three next steps, inserted lowest first, and check the annotation is the highest. Against the current code that test returns the lowest.

Found while benchmarking the overlaps page for SACGF/variantgrid_shariant#394.

Dominant language
Python
Stars
31
Forks
4
Avg merge
8h 15m
Merged PRs (30d)
350

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from SACGF/variantgrid

All issues in SACGF/variantgrid

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.