fix: validate Xenium bundle after UNTAR (drop test-profile skip-validation guard)#159
Merged
heylf merged 5 commits intoMay 20, 2026
Merged
Conversation
…ation guard
## Motivation
Previously `validateXeniumBundle` ran during `PIPELINE_INITIALISATION` and
was gated by `if (!workflow.profile.contains('test'))`. The skip was needed
because:
* The test samplesheet points at a `.tar.gz` URL, not a directory; the
validation iterates required files like `cell_boundaries.csv.gz` over
that path and would always fail in test mode.
* For cloud-storage inputs (`s3://`, `gs://`, `az://`) the validation
also bailed out because `file().exists()` is unreliable before
Nextflow stages the input.
Both reasons describe the same root cause: validation was running BEFORE
staging, so it had to handle multiple un-staged input shapes.
## Change
Move validation to AFTER the `UNTAR` staging step, where the bundle is
unconditionally a resolved local directory (no matter whether the user
provided a directory, a remote tarball, or a cloud URL). This removes
the need for both the test-profile guard AND the cloud-storage early
return — the same validation runs uniformly for production and test.
* `subworkflows/local/utils_nfcore_spatialxe_pipeline/main.nf`:
delete the `validateXeniumBundle(ch_samplesheet)` call and its
`if (!workflow.profile.contains('test')) { ... }` guard. Delete the
`def validateXeniumBundle(ch_samplesheet)` function (it was tightly
coupled to the channel/un-staged-path shape).
* `workflows/spatialxe.nf`: inline the file-presence check into the
existing `ch_bundle_path` `.map` block. Required-file list errors;
optional-file list warns. Validation runs once per sample, after
`UNTAR` (test mode) or directly on the user-provided directory
(production).
## Side effect: UNTAR stub fix
Validating post-staging exposed an existing inconsistency between the
nf-core/untar real script and its stub: the real script uses
`tar --strip-components 1` when the archive has a single top-level
directory, but the stub iterates `tar -tf` output unchanged — which
places stubbed files OUTSIDE `${prefix}/` (at `./xenium_bundle/...`
in our case). The previous local untar patch worked around this by
adding 4 hardcoded `touch ${prefix}/...` lines for the files this
pipeline happens to need first.
The patch is replaced with a proper fix: in the stub's single-
top-level-dir branch, strip the leading directory from each tar
entry so the file layout matches what a real extraction produces.
The 4 hardcoded touches are removed because the loop now creates
them naturally. `modules/nf-core/untar/untar.diff` regenerated.
Snapshots regenerated to reflect the now-complete extracted-bundle
file listing (was previously truncated to the 4 hardcoded files).
## Verification
* `nf-test test tests/{default,coordinate_mode,image_mode,preview_mode,segfree_mode}.nf.test --profile=+docker --ci`
→ 5/5 PASS in ~450s after snapshot regen.
* Workflow runs to `workflow.success == true` against the test
samplesheet (`https://.../xenium_bundle.tar.gz`) — validation
now correctly sees the extracted directory contents.
* No new dependencies. The validation function definition was
deleted from utils_nfcore_spatialxe_pipeline/main.nf since the
inline check in spatialxe.nf is simpler than the old channel-based
iteration.
The legacy `disk=large` label is silently ignored by the current runs-on version — runners get 29 GB total / ~12 GB free, not enough for the cumulative container pull on heavy stub tests. `volume=80gb` (the new syntax) is honored and provisions 76 GB / ~60 GB free. This is the same one-line fix landed in PR nf-core#158 (commit 32f479d); PR nf-core#159 needs it independently since it branched from upstream/dev where the workflow file still has the old label.
…s version Two reconciliations: 1. nf-test version drift. Local devs run nf-test 0.9.5; CI was pinned to 0.9.3. The two versions capture snapshot file metadata differently for gzip-format files (0.9.5 records full File-object shape, 0.9.3 records bare md5 strings), causing PR nf-core#159's locally-regenerated snapshots to mismatch CI's run output. Bumping CI to 0.9.5 (matching the published local default) makes the snapshots round-trip cleanly. 2. Pre-merge harmony with PR nf-core#158. PR nf-core#158 (params-in-modules cleanup) edits `subworkflows/local/utils_nfcore_spatialxe_pipeline/ main.nf` to add `format` plumbing and a method-aware format compatibility check. PR nf-core#159 (this PR) edits the same file to remove the `validateXeniumBundle` call + def. The two edits are in different parts of the file but to ensure PR nf-core#159 cleanly rebases onto PR nf-core#158 (which will merge first), the PR nf-core#158 additions are now present in PR nf-core#159's version of this file. No conflict on the eventual rebase.
Previous commit (8996511) tried to pre-emptively port PR nf-core#158's additions onto PR nf-core#159's subworkflow file. That broke things: PR nf-core#158's subworkflow additions include adding `format` as a take input, but PR nf-core#158's matching change to `main.nf` (passing `params.format` at the call site) was not pulled in (it's PR nf-core#158-only, not overlapping). Result: PIPELINE_INITIALISATION expected 22 take inputs but main.nf passed 21 → `workflow.success = false` in CI. Reverting the subworkflow file to PR nf-core#159's standalone version (removes validateXeniumBundle call + def, no other changes). When PR nf-core#158 merges first, the rebase of PR nf-core#159 will produce a normal 3-way merge on this file — git's auto-merge handles disjoint edits (PR nf-core#158's additions are in different line ranges from PR nf-core#159's removals), so no manual reconciliation is expected.
…apshots nf-test's FileUtil.getMd5() decompresses .gz files before hashing. When the UNTAR stub used `touch foo.gz` (0-byte file), GZIPInputStream threw EOF; the exception handler in PathConverter.java fell through to dumping raw File metadata (path, absolutePath, freeSpace, ...) into the JSON snapshot, causing machine-dependent dict-form entries. Fix: `: | gzip -n > file.gz` produces a 20-byte deterministic empty gzip (magic + empty deflate block, no embedded mtime/filename). Decompressed md5 is d41d8cd98f00b204e9800998ecf8427e on every machine, so snapshots are stable across local and CI runners. All 5 mode tests pass twice locally (regen + validate-without-update).
heylf
approved these changes
May 20, 2026
Collaborator
heylf
left a comment
There was a problem hiding this comment.
I like this. Becomes now cleaner.
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.
Motivation
Previously
validateXeniumBundleran insidePIPELINE_INITIALISATIONand was gated by:```groovy
if (!workflow.profile.contains('test')) {
validateXeniumBundle(ch_samplesheet)
}
```
The skip-in-test guard existed because:
.tar.gzURL, not a directory. The validation iterates required files likecell_boundaries.csv.gzover that path and always fails in test mode.s3://,gs://,az://) also had an early-return becausefile().exists()is unreliable before Nextflow stages the input.Both reasons describe the same root cause: validation was running BEFORE staging, so it had to handle multiple un-staged input shapes (directory, tarball, remote URL).
Change
Move the file-presence validation to AFTER the existing
UNTARstaging step, where the bundle is unconditionally a resolved local directory regardless of input form.subworkflows/local/utils_nfcore_spatialxe_pipeline/main.nfvalidateXeniumBundle(ch_samplesheet)call + its!workflow.profile.contains('test')guard. Delete thedef validateXeniumBundlefunction (it was tightly coupled to the channel + un-staged-path shape).workflows/spatialxe.nfch_bundle_path.mapblock — required-file list errors, optional-file list warns. Same logic, runs once per sample afterUNTAR(test mode) or directly against the user-provided directory (production).The s3/gs/az special-case is no longer needed: by the time
ch_bundle_pathruns its map, Nextflow has staged the input as a local path, sofile().exists()works.Side effect: nf-core/untar stub fix
Running the validation post-staging exposed a latent bug in
nf-core/untar's stub.Real script (when the archive has a single top-level dir):
tar -xavf --strip-components 1— places extracted files inside${prefix}/.Stub script (same condition): iterates
tar -tfoutput andtouch \${i}— but\${i}includes the top-level dir prefix (e.g.xenium_bundle/cells.csv.gz), so the files land at./xenium_bundle/...outside${prefix}/. The captured Nextflow output is only${prefix}/, so most of the iterated touches are lost.The previous local patch in this repo papered over this with 4 hardcoded
touch ${prefix}/morphology.ome.tifetc. lines — enough for the test fixture's first-stage consumers, but the test_run/ directory was still missing 12 of the 16 files defined by 10x's bundle spec, which is what made the skip-validation guard necessary.This PR replaces the band-aid with a proper fix: in the single-top-level-dir branch, strip the leading directory from each tar entry so the stubbed file layout matches the real script. The 4 hardcoded touches are removed since the loop now creates them naturally.
```bash
new stub logic for the single-top-level-dir branch
for i in `tar -tf ${archive}`; do${prefix}/$ (dirname "$stripped")
stripped=${i#*/}
[ -z "$stripped" ] && continue
if [[ ! "${i}" =~ /$ ]]; then
mkdir -p
touch ${prefix}/$stripped
else
mkdir -p ${prefix}/$stripped
fi
done
```
modules/nf-core/untar/untar.diffregenerated to reflect the cleaner change (still a local patch on the nf-core module; upstreaming a separate PR tonf-core/moduleswould let us drop this patch entirely, but that's a separate effort).Snapshot updates
Five mode-test snapshots regenerated to reflect the now-complete extracted-bundle file listing:
tests/coordinate_mode.nf.test.snaptests/default.nf.test.snaptests/image_mode.nf.test.snaptests/preview_mode.nf.test.snaptests/segfree_mode.nf.test.snapEach gained ~15 new entries under
coordinate/untar/test_run/,image/untar/test_run/, etc. — the files that the UNTAR stub now correctly creates inside the prefix (cell_boundaries., cell_feature_matrix., cells., metrics_summary.csv, morphology_focus/, nucleus_boundaries., transcripts.zarr.zip, analysis.tar.gz, analysis.zarr.zip, analysis_summary.html, aux_outputs.tar.gz, .end-of-run).These weren't appearing in the previous snapshots because the broken stub never placed them inside the output directory.
Test plan
nf-test test tests/{default,coordinate_mode,image_mode,preview_mode,segfree_mode}.nf.test --profile=+docker --ci→ 5/5 PASS in 448s after snapshot regen.nextflow config -profile docker .parses clean.nextflow inspect main.nf -profile test,docker— process graph intact.Related
This is the architectural follow-up @awgymer and I discussed on #158 — moving validation to post-staging so the skip-in-test escape hatch can go away. PR #158 is independent (params-in-modules cleanup) and not blocking this.