Sample-bound filter nodes can apply to a patient #1855

[davmlaw]

Issue #1855. Plan: claude/plans/1855_filter_node_patient_scope_plan.md.

Why

Zygosity, Allele Frequency, Mode of Inheritance and Gene List (sample QC panel) hold one sample FK, auto-set from the ancestors' proband. Under a group level SampleNode with two callers on the one extraction (TSO 500 small variants + CNV) the proband sample is ambiguous, so the node stayed unset. Picking one caller by hand was worse: the filter is keyed on that caller's genotype column, which is NULL on every other caller's row, so the node silently dropped them.

What

Each of the four nodes applies to either one sample or one patient. In patient mode the filter applies to every ancestor sample of that patient, ORed as one pk__in subquery per sample (the same shape the group SampleNode produces, and for the same reason: an OR across two genotype aliases has nowhere to hang in annotate_and_filter_queryset). The proband patient auto-sets it where the proband sample is ambiguous. Single-sample analyses keep setting the sample and produce the query they always did.

• patient FK on the four nodes (migration 0144). Exactly one of sample / patient is set.
• AncestorSampleMixin owns the choice (_set_sample / _set_patient, get_filter_samples, get_filter_patient, _get_filter_samples_arg_q_dict). SampleNode's per-sample subquery builder moves to cohort_mixin.get_sample_pk_in_q.
• A sample with no GT (Zygosity, MOI) or no AF column (Allele Frequency) passes through unfiltered, as SampleNode already does; the method summary lists which samples were filtered and which passed through.
• Gene List's sample QC panel unions the patient's samples' active gene lists, resolved when asked rather than stored.
• One "Applies to" picker in the four editors (sample:<pk> / patient:<pk>), mapping a template AnalysisVariable to whichever FK is set.
• The MOI editor's "From Patient" panel now receives every picker choice's gene/disease data at render. This replaces the sample_patient_gene_disease AJAX URL, whose view required an argument the URL never passed.
• Deleting a sample bumps patient mode nodes of that patient.

Tests

FilterNodePatientScopeTest (16 tests) in analysis/tests/test_sample_node_levels.py on the TSO 500 patient fixture. Full suite: 3230 tests OK.

[davmlaw]

🤖 Written by Claude

Open question from #1857: a bare node.sample read is a foot gun.

Zygosity, Allele Frequency, MOI and Gene List nodes now apply to either one sample or one patient (exactly one FK set). In patient mode sample is None, so code that reads it directly doesn't crash - it reads as "unset" and the filter silently doesn't apply. Everything on the branch goes through AncestorSampleMixin.get_filter_samples() / get_filter_patient(), so it's a risk for the next feature, not today's code.

Proposal, to decide later rather than block the PR: rename the two FKs to _sample / _patient, keeping db_column so the migration is metadata only.

• The underscore says "storage, not for you"; pylint flags node._sample outside the class as protected access, and Django templates refuse underscore lookups, so a new bare read is caught by lint or at render rather than by a reviewer.
• Django allows a leading underscore (it only rejects trailing _, __ and pk).
• Cost: eight RenameFields with no SQL, a data step rewriting AnalysisVariable.field for existing template variables on these four node types, the delete-signal lookups (zygositynode___sample - readable enough if each subclass is filtered directly) and a pylint disable on the few form/mixin lines that legitimately touch the fields.

The cheaper alternative is a docstring note above the fields plus is_set on the mixin so nothing outside it has a reason to read them; that removes the temptation but doesn't enforce anything.