Split detail report notebooks by workflow level - #100
Open
KevinMLanderos wants to merge 2 commits into
Open
Conversation
Split the part1/part2 detail reports so each is generated only for the
workflow_level stages that ran, replacing the on/off template swap hack.
template_details_part1.qmd -> template_details_sample.qmd
Rename only. Includes template_sample.qmd; rendered when `sample` runs.
template_details_part2.qmd -> split into:
template_details_compare.qmd — template_overlap.qmd + template_sharing.qmd,
plus a static pointer to the patient report. Rendered when `compare` runs.
template_details_patient.qmd — template_giana.qmd + template_gliph.qmd.
Rendered when `patient` runs; no longer gated behind `compare`.
Drop template_patient_clustering_on/off.qmd and the template_patient_clustering.qmd
staging - the patient report is simply not emitted when the patient stage is absent.
template_giana.qmd: import os in the first cell. It used os.path in early cells
but only imported os later, relying on template_overlap.qmd (which ran first in
the old combined part2) to have imported it. It's now the first include in
template_details_patient.qmd.
Wiring: rename params in nextflow.config / nextflow_schema.json
(template_details_part1 -> _sample; part2 + patient_clustering_on/off ->
_compare + _patient), rework the report channels in
subworkflows/local/bulktcr_analysis.nf, and update the README report table.
Verified with nf-test: Bulk {sample,compare}, {sample,patient},
{sample,patient,compare} all pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WhjujvKjf1RV5gABqZnMFB
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restructures the bulk HTML “Details” reporting so that sample-, compare-, and patient-level detail notebooks are generated only for the workflow stages that actually ran, removing the previous patient-clustering on/off template swap approach.
Changes:
- Renames/splits detail report templates into
template_details_sample.qmd,template_details_compare.qmd, andtemplate_details_patient.qmd, and updates the compare notebook to point readers to the separate patient report. - Rewires report channel construction in
BULKTCR_ANALYSISto emit the new notebooks at the appropriate workflow levels (and decouples patient details fromcompare). - Updates pipeline params/schema/docs accordingly and removes the obsolete patient clustering placeholder templates.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| subworkflows/local/bulktcr_analysis.nf | Updates report emission wiring to use split sample/compare/patient detail templates. |
| README.md | Updates report documentation to reflect the split detail notebooks and when they are rendered. |
| notebooks/template_patient_clustering_on.qmd | Removes obsolete patient-clustering “on” wrapper template. |
| notebooks/template_patient_clustering_off.qmd | Removes obsolete patient-clustering “off” placeholder template. |
| notebooks/template_giana.qmd | Adds an early import os to prevent hidden dependency on other included notebooks. |
| notebooks/template_details_sample.qmd | Adds the new sample-level details notebook wrapper including template_sample.qmd. |
| notebooks/template_details_patient.qmd | Refocuses patient details to include GIANA/GLIPH2 content directly. |
| notebooks/template_details_compare.qmd | Adds new comparison-level details notebook wrapper; includes overlap/sharing and points to patient report. |
| nextflow.config | Renames report template params and removes patient-clustering on/off params. |
| nextflow_schema.json | Renames/updates report template params and their descriptions. |
Suppressed comments (1)
README.md:82
- This bullet says including
patientin--workflow_levelrenders the patient details notebook, but report generation is sample-centric and currently requiressampleto be present. Wording should match actual behavior to avoid confusing users who run--workflow_level patientwithoutsample.
- `--workflow_level` includes `patient` → renders `template_details_patient.qmd` (patient-level clonotype analysis)
- `--use_gliph2` → additionally includes GLIPH2 clustering in the patient details report
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+72
to
+74
| | `template_details_sample.qmd` | `sample` | Detailed per-sample repertoire analysis | | ||
| | `template_details_compare.qmd` | `compare` | Detailed cross-sample / longitudinal comparison analysis | | ||
| | `template_details_patient.qmd` | `patient` | Patient-level clonotype clustering (GIANA, and GLIPH2 when `--use_gliph2`) | |
Comment on lines
+135
to
138
| "template_details_patient": { | ||
| "type": "string", | ||
| "description": "Path to patient-level clustering (GIANA/GLIPH2) notebook template, used when patient workflow_level is run." | ||
| "description": "Path to the patient-level details notebook template with GIANA/GLIPH2 clustering (rendered when the patient workflow_level is run)." | ||
| }, |
Comment on lines
+114
to
+116
| Patient-level clonotype clustering (GIANA and GLIPH2) is reported separately in | ||
| the **Patient Details** report (`template_details_patient.html`), which is | ||
| generated when `patient` is included in `--workflow_level`. |
Comment on lines
+211
to
+213
| // Patient-level details report (GIANA, plus GLIPH2 when --use_gliph2). Rendered | ||
| // whenever the patient stage ran; it does not depend on the compare stage. | ||
| if (levels.contains('patient')) { |
Bulk HTML report rendering is gated on run_reports (levels.contains 'sample'); the README table/bullet, nextflow_schema.json descriptions, the compare notebook's callout, and a code comment all implied the compare/patient details reports render whenever compare/patient ran, without the sample requirement. Flagged by Copilot in PR #100 review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Split the part1/part2 detail reports so each is generated only for the workflow_level stages that ran, replacing the on/off template swap hack.
template_details_part1.qmd -> template_details_sample.qmd
Rename only. Includes template_sample.qmd; rendered when
sampleruns.template_details_part2.qmd -> split into:
template_details_compare.qmd — template_overlap.qmd + template_sharing.qmd,
plus a static pointer to the patient report. Rendered when
compareruns.template_details_patient.qmd — template_giana.qmd + template_gliph.qmd.
Rendered when
patientruns; no longer gated behindcompare.Drop template_patient_clustering_on/off.qmd and the template_patient_clustering.qmd staging - the patient report is simply not emitted when the patient stage is absent.
template_giana.qmd: import os in the first cell. It used os.path in early cells but only imported os later, relying on template_overlap.qmd (which ran first in the old combined part2) to have imported it. It's now the first include in template_details_patient.qmd.
Wiring: rename params in nextflow.config / nextflow_schema.json (template_details_part1 -> _sample; part2 + patient_clustering_on/off -> _compare + _patient), rework the report channels in subworkflows/local/bulktcr_analysis.nf, and update the README report table.
Verified with nf-test: Bulk {sample,compare}, {sample,patient}, {sample,patient,compare} all pass.