Single-cell: interactive master summary, TCRdist3 + TCRi fixes, GIANA clonotype dedup - #99
Open
zaidissa wants to merge 6 commits into
Open
Single-cell: interactive master summary, TCRdist3 + TCRi fixes, GIANA clonotype dedup#99zaidissa wants to merge 6 commits into
zaidissa wants to merge 6 commits into
Conversation
zaidissa
changed the title
zaidissa
force-pushed
the
singlecell-master-summary
branch
from
August 31, 2026 16:32
117b8d9 to
317e4c7
Compare
… clonotype dedup Single-cell modality work. Both routes verified end to end on 8 samples / 2 patients: VDJ-only 123 tasks, full SC 126 tasks, 0 failures, 12/12 modules contributing. ## Fixes **TCRdist3 was silently dropped from every full-SC run.** `enrich_seurat.R` required `rhdf5`, which is absent from the single-cell container (only `hdf5r` is present), so every sample fell through to a `.csv` branch that never exists in sparse mode. Because `tcrdist_export` is `optional: true`, nothing failed — the method just vanished. Now reads via whichever HDF5 package is available: **0 → 22,496 cells annotated (27%)**. **TCRi was dead code.** `modules/scratch/TCRI/` and `subworkflows/scratch/tcri.nf` existed but nothing included them, while `run_tcri = true` in the config implied otherwise. Wired `TCRI_SW` into the SC workflow, added the missing `tables`/`figures` emits, and guarded an unbound `LD_LIBRARY_PATH` that aborted the task under `set -u` before any R ran. **17,843 / 18,610 cells scored (95.9%).** **Master Summary rebuilt.** 21 of the 30 file params the `.qmd` declared were never passed, so 7 sections rendered blank; `stageAs` was also stripping file extensions, so no figures embedded at all. Replaced the ~30 hand-wired params with per-module table staging (`intables/<module>/*`), so new upstream tables no longer need process-input changes. Now 17 interactive plotly figures and 12 sortable tables. Figures use native `plot_ly()` rather than `ggplotly()`, which is broken against ggplot2 4.x in this container. **New `CLUSTER_ROLLUP` step.** GIANA/GLIPH2/TCRdist3 produced raw per-patient output but no rollup, so those modules always reported absent. Adds rollups plus method coverage, and radius-based TCRdist clustering for the VDJ-only route. **`VDJ_QC`**: added `pairing_bar_by_sample`, which the Master Summary requested by name but no module produced. ## Please review: one change reaches the bulk engine `modules/local/compare/giana.nf` — collapses to one row per clonotype before GIANA. `PATIENT_CONCATENATE` pools a patient's samples by stacking rows, so GIANA was clustering cross-sample duplicates rather than similar sequences: **76 of 77 clusters on the single-cell test set contained a single distinct CDR3b**, and single-sample patients produced no output at all. This affects bulk too, so it is **gated behind `giana_dedup_clonotypes`, default `false`** — bulk results are unchanged unless explicitly enabled. `params_singlecell.yml` sets it true. Happy to drop the change entirely if you would rather handle it separately. No other bulk-side file is modified, and nothing is deleted. ## Not covered - Bulk mode has not been executed against this branch (`-preview` only). The gate makes it a no-op by default, but it is unverified. - No regression tests added. The `rhdf5` issue in particular is invisible when it breaks. - Tested on one dataset only.
zaidissa
force-pushed
the
singlecell-master-summary
branch
from
August 31, 2026 16:32
317e4c7 to
c50f985
Compare
- process-form.json / process-input.json: add patient_col, giana_dedup_clonotypes and tcrdist_radius to both single-cell profiles; run_tcri on full_gex only (it needs a GEX object). - preprocess.py: warn when the sample sheet has no patient column. PATIENT pools samples by it for GIANA/GLIPH2, so without it every sample becomes its own patient and the clustering is silently wrong. Also logs the resolved sample/patient counts. The check is defensive and cannot fail preprocessing.
The VDJ-only route emits no per-cell object. BULK_TO_EXPORT synthesizes a clonotype-level export whose cells are reconstructed rather than real barcodes (4,372 synthetic rows against 23,764 actual cells on the 8-sample test set). Builds a merged object from VDJ_QC's contig tables at both stages, keeping real barcodes: a Seurat object with 27 TCR metadata columns, the scRepertoire combineTCR object, and flat per-cell and per-sample tables. The pre/post pair makes QC's effect directly measurable. cellranger vdj has no genes, so the Seurat counts assay is a zero placeholder; this is recorded in seu@misc$provenance. Runs on both routes, toggleable via run_merge_vdj_object. Also narrows the run-artifact ignore rule from work*/ to work/ and work_*/ - the wildcard form also matched workflows/, the source directory.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the single-cell (SC) pipeline route by restoring previously missing analyses (notably TCRdist3 and TCRi), rebuilding the Master Summary report input model to be module-table–driven, and adding new “bridge” rollups/artifacts to make SC outputs complete and comparable across routes. It also introduces an optional GIANA clonotype deduplication step (default off) that can affect bulk behavior only when explicitly enabled.
Changes:
- Wire
TCRI_SWinto the full-SC workflow, fixLD_LIBRARY_PATHunderset -u, and re-export TCRi tables/figures for downstream reporting. - Rework Master Summary inputs from ~30 individually-wired params to per-module staged
tables/directories (and update the QMD accordingly), plus add new cluster rollup staging for GIANA/GLIPH2/TCRdist3. - Add optional GIANA per-patient deduplication (
giana_dedup_clonotypes) to prevent cross-sample duplicate rows inflating “clusters”; enable it for single-cell viaparams_singlecell.yml.
Reviewed changes
Copilot reviewed 18 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| workflows/tcrtoolkit_sc.nf | Wires TCRi into SC workflow; stages per-module tables into Master Summary; adds cluster rollups + merged VDJ object step. |
| subworkflows/scratch/tcri.nf | Re-exports TCRi tables/figures for consumption by the Master Summary workflow. |
| subworkflows/scratch/master_summary.nf | Updates Master Summary subworkflow interface to accept per-module table collections. |
| params_singlecell.yml | Enables giana_dedup_clonotypes for SC runs. |
| nextflow.config | Adds giana_dedup_clonotypes, run_merge_vdj_object, and tcri_scores_file params. |
| modules/scratch/VDJ_QC/VDJ_QC_analysis.qmd | Adds pairing-by-sample bar plot that the report expects. |
| modules/scratch/TCRI/main.nf | Fixes LD_LIBRARY_PATH expansion under set -u. |
| modules/scratch/MASTER_SUMMARY/Master_Summary_Report.qmd | Major rebuild: module-table directory model + interactive Plotly/Reactable report rendering. |
| modules/scratch/MASTER_SUMMARY/main.nf | Stages per-module tables/ directories under intables/<module>/ and updates Quarto params. |
| modules/local/compare/giana.nf | Adds optional clonotype-row deduplication prior to GIANA (gated by giana_dedup_clonotypes). |
| modules/bridges/merge_vdj_object.nf | New bridge process to build pre/post-QC per-cell VDJ object outputs. |
| modules/bridges/cluster_rollup.nf | New bridge process to generate rollup tables for cluster methods and method coverage. |
| bin/merge_vdj_object.R | New script to build combineTCR + placeholder Seurat + per-cell TSV from contigs. |
| bin/enrich_seurat.R | Fixes HDF5 reading by supporting hdf5r when rhdf5 is unavailable. |
| bin/cluster_rollup.py | New script to generate GIANA/GLIPH2/TCRdist3 rollups and method coverage tables. |
| .gitignore | Ignores additional run artifact directories; un-ignores notebooks/template_*.qmd. |
| .cirro/singlecell_vdjonly/process-input.json | Adds patient_col, GIANA dedup toggle, and tcrdist_radius to Cirro wiring. |
| .cirro/singlecell_vdjonly/process-form.json | Adds UI fields for patient_col, GIANA dedup toggle, and tcrdist_radius. |
| .cirro/singlecell_vdjonly/preprocess.py | Adds preprocessing warnings/info around missing/NA patient column. |
| .cirro/singlecell_full_gex/process-input.json | Adds patient_col, run_tcri, GIANA dedup toggle, and tcrdist_radius to Cirro wiring. |
| .cirro/singlecell_full_gex/process-form.json | Adds UI fields for patient_col, run_tcri, GIANA dedup toggle, and tcrdist_radius. |
| .cirro/singlecell_full_gex/preprocess.py | Adds preprocessing warnings/info around missing/NA patient column. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The Copilot Autofix commit (2bea78f) that changed the dedup_clonotypes truthiness check from ?: to == true also dropped the script: label above it. Without it, Nextflow's parser stays in output-declaration mode and treats the def statement as an output entry, failing with "Invalid process output" at giana.nf:16 - this broke script compilation for every bulk test touching patient/compare (16 of 17 integration tests failed in PR #99's CI run). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The three clustering modules write raw per-patient output and no figures - the bulk notebooks that plot them (template_giana.qmd, template_gliph.qmd) render only in bulk mode, so on the single-cell route GIANA and GLIPH2 computed correctly but nothing displayed them. CLUSTER_ROLLUP now emits five detail tables read straight from that raw output: per-cluster membership for each method, and V-gene usage among clustered clonotypes. Each cluster row carries n_members, n_unique_cdr3 and n_samples, so a cluster whose members are all one CDR3b is visibly duplicate detection rather than similarity. Master Summary gains four figures and a table: cluster size distribution across methods, top GLIPH2 motif clusters, the fraction of clusters spanning more than one sample, the GIANA cluster table, and V-gene usage. 13 -> 17 interactive figures. On the 8-sample test set: 89/89 GLIPH2 motifs and 7/7 GIANA clusters contain more than one distinct CDR3b, and roughly three quarters of clusters from both methods span multiple samples.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Single-cell modality work. Both routes verified end to end on 8 samples / 2 patients:
VDJ-only 123 tasks, full SC 126 tasks, 0 failures, 12/12 modules contributing.
Fixes
TCRdist3 was silently dropped from every full-SC run.
enrich_seurat.Rrequiredrhdf5, which is absent from the single-cell container (onlyhdf5ris present), so everysample fell through to a
.csvbranch that never exists in sparse mode. Becausetcrdist_exportisoptional: true, nothing failed — the method just vanished.Now reads via whichever HDF5 package is available: 0 → 22,496 cells annotated (27%).
TCRi was dead code.
modules/scratch/TCRI/andsubworkflows/scratch/tcri.nfexistedbut nothing included them, while
run_tcri = truein the config implied otherwise. WiredTCRI_SWinto the SC workflow, added the missingtables/figuresemits, and guarded anunbound
LD_LIBRARY_PATHthat aborted the task underset -ubefore any R ran.17,843 / 18,610 cells scored (95.9%).
Master Summary rebuilt. 21 of the 30 file params the
.qmddeclared were never passed,so 7 sections rendered blank;
stageAswas also stripping file extensions, so no figuresembedded at all. Replaced the ~30 hand-wired params with per-module table staging
(
intables/<module>/*), so new upstream tables no longer need process-input changes.Now 17 interactive plotly figures and 12 sortable tables. Figures use native
plot_ly()rather than
ggplotly(), which is broken against ggplot2 4.x in this container.New
CLUSTER_ROLLUPstep. GIANA/GLIPH2/TCRdist3 produced raw per-patient output but norollup, so those modules always reported absent. Adds rollups plus method coverage, and
radius-based TCRdist clustering for the VDJ-only route.
VDJ_QC: addedpairing_bar_by_sample, which the Master Summary requested by name butno module produced.
Please review: one change reaches the bulk engine
modules/local/compare/giana.nf— collapses to one row per clonotype before GIANA.PATIENT_CONCATENATEpools a patient's samples by stacking rows, so GIANA was clusteringcross-sample duplicates rather than similar sequences: 76 of 77 clusters on the
single-cell test set contained a single distinct CDR3b, and single-sample patients
produced no output at all.
This affects bulk too, so it is gated behind
giana_dedup_clonotypes, defaultfalse—bulk results are unchanged unless explicitly enabled.
params_singlecell.ymlsets it true.Happy to drop the change entirely if you would rather handle it separately.
No other bulk-side file is modified, and nothing is deleted.
Not covered
-previewonly). The gate makes it ano-op by default, but it is unverified.
rhdf5issue in particular is invisible when it breaks.