Stop skip-flag guards becoming reference rebuilds and assets

[jmchilton]

Posted by Claude (AI assistant) on behalf of @jmchilton — not authored by them personally.

Closes #355.

Root cause

detectReferenceRebuilds filtered skip-flag guards by testing the raw negated identifier. nf-core subworkflows conventionally rename take slots, so bacass's if (!val_skip_fastqc) (subworkflows/nf-core/fastq_trim_fastp_fastqc/main.nf:39, take declared val_skip_fastqc // value: boolean) sailed past the anchored ^(skip|no|disable|disabled)_. resolveAssetParam then resolved the rule back to the real skip_fastqc param via the caller's positional binding in workflows/bacass.nf:176 — with confidence: high. buildReferenceAssets admitted it unfiltered, because the rebuild path bypasses both isNonReferenceParam and isPathTypedParam.

Fix

Both ends, as the issue asks:

• Rule side — detectReferenceRebuilds now takes params: Param[] rather than a bare name set, and drops a rule when the resolved asset_param is boolean-typed (or skip-named).
• Asset side — buildReferenceAssets rejects a boolean param however it was nominated.

The raw-identifier pre-filter stays. Instrumenting it across the corpus showed it fires 101 times catching the direct !params.skip_* and bare !skip_* forms — it's live, just not sufficient alone. It's also defeated in shapes no prefix-stripping would catch: domain-namespaced flags (arg_skip_abricate, bgc_skip_antismash, mapstats_skip_preseq), skip with no trailing underscore (preprocessing_skippairmerging, longread_qc_skipadaptertrim), and an adjacent class the vocabulary never covered (!params.run_taxa_classification, !params.with_umi, !build_only_index). Those don't currently produce rules only because they fail later gates — luck, not design. So the resolved-param type check is the durable defense; the name filter is a cheap first pass.

Deviation from the suggested acceptance criteria

The issue suggests requiring the param be path-typed on the rebuild path. That would break correct output: bacass's baktadb is type: string, format: null — not path-typed — as are all five eager assets (fasta_fai, fasta_dict, fasta_mapperindexdir). Only sarek's 6 declare file-path/directory-path. A path-typed gate would drop 6 of the corpus's 11 legitimate rebuild assets, including the baktadb the issue's own criterion requires. Gated on non-boolean instead.

Tests

Three, all verified red-to-green:

1. resolver.test.ts — "skips skip-flag guards renamed by a subworkflow take slot": synthetic pipeline mirroring bacass, alongside a legitimate fasta_fai rebuild, asserting only the legitimate rule and assets survive. Pre-fix: 2 rules.
2. resolver.test.ts — "keeps a boolean param out of reference_assets when a rebuild nominates it": pins the fallback_for path, which is independently reachable — the rule legitimately stands with fallback_for: "build_index" and the asset gate must still block it. Pre-fix assets: ['build_index', 'fasta', 'fasta_fai']. The asset-side fix is load-bearing, not just belt-and-braces.
3. integration/cli.test.ts — "rebuilds only the Bakta database, not the FASTQC/FASTP skip flags": the real bacass fixture, pinning the headline criterion.

Verification

• bacass: 4 rules / 3 assets → 1 / 1 (baktadb / BAKTA_BAKTADBDOWNLOAD).
• sarek (6 rules / 48 assets) and eager (5 / 11) unchanged; every other fixture byte-identical on both fields. Swept all 18 materialized pipelines — zero boolean rules or assets remain corpus-wide.
• summarize-nextflow 90/90, root npm run test 151/151, packages-typecheck, packages-lint, packages-format, validate (0 errors) pass. cast summarize-nextflow --check clean — no bundle regeneration needed.

🤖 Generated with Claude Code