[sofiademmou]

PR checklist

This PR adds a new nf-core module for kivvi.
Closes #13089

• This comment contains a description of changes (with reason).
• If you've fixed a bug or added code that should be tested, add tests!
• If you've added a new tool - have you followed the module conventions in the contribution docs
• If necessary, include test data in your PR.
• Remove all TODO statements.
• Broadcast software version numbers to topic: versions - See version_topics
• Follow the naming conventions.
• Follow the parameters requirements.
• Follow the input/output options guidelines.
• Add a resource label
• Use BioConda and BioContainers if possible to fulfil software requirements.
• Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
  • For modules:
    • nf-core modules test <MODULE> --profile docker
    • nf-core modules test <MODULE> --profile singularity
    • nf-core modules test <MODULE> --profile conda
  • For subworkflows:
    • nf-core subworkflows test <SUBWORKFLOW> --profile docker
    • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
    • nf-core subworkflows test <SUBWORKFLOW> --profile conda

[erikrikarddaniel]

Reviewed with Claude Code.

Thanks for adding kivvi!
I ran v1.1.0 from the module's container on the two example bamlets (twice for kiv2, to check which outputs are stable).

The main thing: the module only emits the realigned BAM, but kivvi's results are the JSON report (copy number, alleles, variants) and the VCF; it also writes an SVG and a BAM index.
Suggestions inline for main.nf and the stub; meta.yml needs the same outputs.

Tests:

• The real tests read their bamlets from the main branch of the tool's own repository; they should come from nf-core/test-datasets.
• The JSON's supporting_reads lists change order between runs, so the JSON (and the SVG) need unstableKeys, and the copy number call itself is worth asserting (values inline).
• The "set outdir" stub test passes --out ./, which the module already sets, so it can't fail on its own.

Minor: the PR checklist is still all unticked.

[erikrikarddaniel]

Reviewed with Claude Code.

Thanks for the changes. The JSON, VCF, SVG and BAI outputs are emitted and the stub matches the real file names. The tests now read subsets of the example bamlets from nf-core/test-datasets, with their provenance documented in the data PR. The JSON and SVG are in unstableKeys, and the tests assert the copy number (28 for D4Z4, 11 for KIV2) and the SVG. I ran all four tests twice with docker at NXF_VER=26.04.0: they pass both times, so the BAM, VCF and BAI md5s are stable.

One point to fix before merge, inline: kivvi d4z4 accepts only 1 or 2 for --threads, and the module passes task.cpus as is. With the default process_single label that is 1, but a config that gives the process more than 2 CPUs makes every D4Z4 run fail with D4Z4 flanking thread count must be 1 or 2. I reproduced it with cpus = 4 in the test config. Capping the value fixes it, and with the cap all four tests pass at cpus = 4, with the snapshots unchanged.

Smaller points, also inline:

• nf-core modules lint warns has_meta_containers; nf-core modules containers create kivvi adds the containers: block.
• The d4z4 - set outdir - stub test is now a plain D4Z4 stub, so the name can change.
• Two closing brackets in the test inputs sit on the same line as the file.

[erikrikarddaniel]

Reviewed with Claude Code.

Thanks for the changes. The --threads cap, the process_low label, the containers: block and the renamed stub test are all in. Lint passes with no warnings, and I ran all four tests with docker at NXF_VER=26.04.0: they pass, and the real-test md5s are unchanged with the rebuilt image.

Two small points, inline:

• The Math.min(task.cpus as int, 2) cap has no test: with cpus = 2 in the test config, removing the cap keeps every test green. With cpus = 4 and the cap removed, bam - d4z4 fails with D4Z4 flanking thread count must be 1 or 2, and with the cap all four pass. Setting cpus = 4 there makes the test fail if the cap is lost.
• The generated container ternary starts its lines with ? and : without indentation; the suggestion puts it in the usual shape.

The cap test is non-blocking, but I'd add it.