Make orf_class purely positional and add uoORF, doORF and intORF - #226
Merged
Conversation
The classifier matched caller ORF-type labels by substring, so `overlap_uORF` matched the `uorf` rule and RiboCode's and ribotricer's CDS-overlapping forms were indistinguishable from plain uORFs. Every caller's vocabulary is a closed enum, so match tokens exactly instead, and report a count of unmapped tokens on the provenance line so an upstream label change fails loudly rather than landing in the wrong class by accident. orfmerge now partitions rows by clustering strategy rather than by class, with `orf_class` absent from every grouping key. Callers disagree on class for the same ORF -- Ribo-TISH reports `5'UTR` for both uORFs and CDS-overlapping uORFs because tisType() only tests the start position -- so keying on class would emit one catalogue row per disagreeing caller. Representative selection resolves the disagreement instead, via CLASS_SPECIFICITY. The clustering enumeration was closed, so a class absent from the list was dropped from the catalogue silently. It now aborts on an unknown class and on any unassigned row. novel_u and smORF cluster in one pass rather than two, so a novel ORF that two callers size either side of 100 aa can now merge. Adds orf_type_native, carrying each caller's own label through to the catalogue, so harmonisation decisions stay auditable without re-running callers. No orf_class value changes in this commit.
reclassify_smorf() replaced orf_class with "smORF" for every ORF at or under 100 aa, so a short uORF, a short downstream ORF, a short truncated CDS variant and a short novel intergenic ORF were indistinguishable in exactly the size range microprotein work cares about. Because orfmerge keys its clustering strategy on orf_class and orfcollapse scoped its peptide dedup to the smORF value, the length override also decided how an ORF merged across callers. orf_class is now purely positional and length lives in a separate is_smorf flag, set from the new --smorf-max-aa (default 100, reproducing the previous membership exactly). Changing the threshold moves is_smorf without moving a single orf_class value. The vocabulary gains uoORF, doORF and intORF, which the callers were already reporting: RiboCode and ribotricer emit Overlap_uORF/Overlap_dORF, RiboCode and Ribo-TISH emit internal, and PRICE emits uoORF and iORF. PRICE's orphan now maps to novel_u rather than other. ribotricer's internal stays other, because it is check_orf_type()'s terminal fall-through rather than a frame-tested call, and labelling it intORF would assert a frame relationship the tool never tested. orfcollapse derives its eligible set from its own aa_length column plus --smorf-max-aa instead of consuming a propagated flag, and aborts if that column is missing. Deriving it locally means the collapse scope cannot silently disagree with the published is_smorf. canonical_cds rows are now overlap-clustered within their transcript. Grouping on (transcript_id, strand) alone folded a short truncated variant into the full-length CDS and emitted only the longest, which the smORF override had been masking; without the fix, removing the override would have deleted those variants from the catalogue. Catalogue row counts move in both directions -- ORFs whose callers disagreed on class now merge, and short CDS variants no longer vanish. Documented as breaking in the CHANGELOG with the migration for orf_class == "smORF".
Reciprocal overlap was measured on the outer genomic span, which for a spliced ORF is mostly intron, so two ORFs differing by a quarter of their codons could still score above 0.99. It is now measured on summed exon-block intersection, taken from the BED12 blocks the merger already loads. Clustering also used single linkage, which chained biologically distinct ORFs together through an intermediate that overlapped both: on a synthetic fixture one catalogue row absorbed a uORF, a uoORF, a doORF, an internal ORF and an extended CDS. Linkage is now complete -- a row joins only if it meets the threshold against every existing member. Transcript-anchored classes stay on the exact span key. Overlap clustering was tried for them and merges nested ORFs: a 100 aa uORF covering 84% of its transcript's CDS folded into the CDS. Distinguishing "same ORF, fuzzy bounds" from "different ORF, mostly nested" needs a shared-stop-codon test rather than an overlap fraction, which is left for a follow-up. The consequence -- two callers whose short-ORF bounds differ by a few nucleotides yield one row each -- is documented in the CHANGELOG and pinned by a test. orfcollapse now prefers the more specific class when a peptide cluster spans several, mirroring orfmerge. Previously the survivor was chosen by length alone, so a 40 aa annotated CDS was deleted by a 60 aa novel ORF sharing its peptide and the MultiQC table reported zero canonical_cds. Adds the five tests the plan specified but did not have: class-disagreement merging, canonical_cds variant separation, the pinned short-ORF de-merge, the is_smorf boundary at smorf_max_aa, and a non-default --smorf-max-aa override. The orfcollapse collapse-happened assertion compared against a constant that the assertion above it already implied; it now compares against the input row count. The is_smorf equivalence asserted `aa_length <= 100`, which is false for a 0 aa ORF. Also states in orfnormalise/meta.yml that Ribo-TISH's AALen is consumed verbatim while every other caller derives aa_length as (nt - 3) // 3, so is_smorf may differ by one residue at the boundary for Ribo-TISH. Not reconciled -- it needs a run whose nucleotide lengths can be compared against AALen.
The module fixtures live on S3, which is unreachable here, so these tests had never executed. Pointing `modules_testdata_base_path` at the test-datasets fork branch (nf-core/test-datasets#2193) over https and running the pure-Python templates without a container makes them runnable, which surfaced three defects. `def native` shadowed a reserved Java keyword, so the whole orfmerge test file failed to compile and every test in it was silently unrunnable from the moment it was introduced. Renamed to `native_col`. `params` is not in scope in an nf-test `then` block, so the collapse-happened assertion resolved its fixture path to `nullgenomics/...` and threw. The row count it compared against was a constant the assertion above it already pinned, so it is now a comment on that assertion instead. A synthetic fixture row carried Ribo-TISH's `5'UTR` label inside a single-quoted Groovy string, whose apostrophe terminated the literal early. The cross-caller merge guard is dropped: it cannot pass on this fixture, whose Ribo-TISH and RiboCode calls never share a transcript and span. Confirmed by injecting the same assertion into dev's test file, where it also fails, so this is a pre-existing gap in the fixture rather than a regression. The synthetic class-disagreement test covers the behaviour deterministically. With those fixed, every behavioural assertion passes: all five new tests, all orfnormalise caller tests including `unmapped_orf_type=0` on real chr20 data, and thirteen of orfcollapse's fourteen. The BED12 outputs are byte-identical to dev for every caller, confirming no coordinates moved. The only remaining failures are `snapshot(process.out).match()`, which needs re-recording inside the module container -- versions.yml recorded against a host Python would be wrong.
nf-core/modules#12495 added two synthetic cross-caller merge tests to custom/orfmerge, written against the pre-change schema. orfmerge now requires orf_type_native and is_smorf on its input, so their inline fixtures gain both columns; without them the module's presence guard aborts. Ribo-TISH's 5'UTR label carries an apostrophe, so those literals are double-quoted. Snapshots are re-recorded for all three modules. Content md5s come from a local run, which is byte-identical to CI for deterministic outputs; the versions.yml md5s are kept from the committed snapshots, because those carry the container's interpreter version rather than the host's. All 18 tests now differ from their snapshots on versions.yml alone, which is the expected consequence of running without the module container.
…napshot The --smorf-max-aa override test references tests/smorf_max_aa.config, which was never added upstream, so the test failed on every engine with "The specified configuration file does not exist". orftable_fasta_gtf_buildorfcatalogue composes all three modules, so its snapshot moves with them. Its chain includes bedtools/getfasta, seqkit/translate and mmseqs/easycluster, none of which can run here, so the values come from CI, whose diffs were pure md5 substitutions with no structural change. Its old catalogue.bed12 md5 matches what nf-core/modules#12495 re-recorded for orfmerge, independently confirming that value. The subworkflow's own main.nf and meta.yml are deliberately left alone: upstream has since corrected a comment there, and shipping the pinned copy would revert it.
…lapse Class selects the clustering strategy in orfmerge, so two callers agreeing on an ORF's exact structure but disagreeing on its class were partitioned into different strategies and could never merge -- the de-merge this PR set out to remove, surviving one level down. Rows sharing an identical exon structure are now grouped first and clustered as a single proxy chosen by class specificity, with the members restored afterwards. Output is byte-identical on every existing fixture, so a synthetic test pins the new behaviour. orfcollapse takes eligibility from the catalogue's is_smorf rather than re-deriving it, and requires the column to be present and 0/1. --smorf-max-aa is kept solely to re-derive the flag and abort when the two disagree, so a threshold passed to one process and not the other fails loudly instead of silently narrowing the collapse scope. This keeps the flag and the length on a row in agreement, which deriving is_smorf from the row's own aa_length needs. orfnormalise aborts on an unmapped ORF-type token instead of counting it: the token falls through to `other` and selects the wrong merge strategy. Explicit `other` mappings are unaffected, since they report as matched. Also corrects two shipped docs against the code: orfcollapse's representative is chosen by class specificity before length, and orfmerge's reciprocal overlap is measured on summed exon-block intersection, not the outer genomic span. The two one-off orfnormalise test configs are replaced by a shared tests/nextflow.config exposing params.module_args.
The exact-structure pre-grouping moved the unknown-class guard onto proxies. representative() ranks an unknown class last, so at a shared structure a known class always won the proxy and the unknown one escaped the guard entirely, then write_catalogue re-emitted the row under the known class -- the silent swallow the guard exists to prevent. Validate the input rows before grouping. Also states orfnormalise's fail-fast behaviour in meta.yml: an unmatched token aborts before outputs are written, so a non-zero unmapped_orf_type can never appear in the provenance line.
nf-core/modules#12498 merged as c804473fd, so point the three custom/orf* modules and orftable_fasta_gtf_buildorfcatalogue at it. Module and test files were already byte-identical to the merged commit, so only the four git_sha values move. The subworkflow's main.nf and meta.yml were not identical: #12498 shipped only that subworkflow's snapshot, so its local copies still carried the pre-merge ch_gtf wording and check_local_copy failed against the new pin. Take upstream's copies, which are the more accurate ones — they record that rpbp and price do resolve gene ids from the GTF and that rpbp has no fallback, where the local text claimed both ignored it. One upstream comment is now stale as a result: main.nf still describes the collapse scope as `aa_length <= 100`, which #12498 made configurable via --smorf-max-aa. That needs a follow-up upstream, not a local divergence. The git_sha values were edited by hand. Every `nf-core modules lint` and `nf-core subworkflows lint` invocation rewrites modules.json wholesale with different formatting (420 lines), so the file was restored and the four values re-applied after linting rather than before.
Bumps the four git_sha values to be91fb1578e47bf1a8acc301bf7d755cb7313fc2 and syncs the three custom/orfnormalise files that #12613 changed: meta.yml, the template, and the module test. The sync is the load-bearing half. A git_sha bump asserts that the local copy equals that commit's copy, and a merge is precisely what changes it -- so bumping alone would have failed check_local_copy on those three files, the same way #12498 did on 08-06 when it shipped a subworkflow snapshot but not its main.nf. orfmerge, orfcollapse and the subworkflow were already byte-identical to the new commit, so only their pins moved. Verified by comparing git blob hashes against the merge commit's tree: all 27 files across the four entries match, and modules.json changed on exactly four git_sha lines with its 101 entries and 534 lines otherwise untouched.
…bulary Recorded by run 31404995990 on ci/record-orf-class-snapshots at 4e49803, which sits on top of the fifth pin, so these describe the code being shipped. Every md5 that moved belongs to a file carrying orf_class -- all six SRX*.ribotish.normalised.tsv, the cohort catalogue and its consensus products, and the psite counts keyed on catalogue ORF ids. The ribotish BED12s did not move, which is the discrimination to expect from a classification-only change: coordinates are unaffected, only the sidecar class column changes. Also corrects a tooling mismatch. The previous snapshots recorded nf-test 0.9.5 and Nextflow 26.04.6, while .github/workflows/nf-test.yml pins 0.9.4 and 25.10.4; these were recorded on those pinned versions, so they match the leg that validates them. Note the class values themselves are not verifiable from a pipeline snapshot -- it stores md5s and tool versions, not file contents. The mapping is proven instead by the module tests, where all seven colon-qualified Ribo-TISH tokens classify correctly with zero unmapped.
The seven new bullets were written before the PR existed, so they lacked the [#NNN](url) prefix and author attribution this changelog uses throughout.
|
Warning Newer version of the nf-core template is available. Your pipeline is using an old version of the nf-core template: 4.0.3. For more documentation on how to update your pipeline, please see the Synchronisation documentation. |
|
❌ nf-test failed with latest Nextflow versionNote Tests with Nextflow's latest version failed but it will not cause a CI workflow failure.
See the full run for details. |
This test arrived on dev via #225 carrying a snapshot recorded against the previous vocabulary, so it was the one ORF catalogue test #226 had not re-recorded and the only failing shard. Its pipeline completed successfully; only the snapshot assertion failed. Recorded by run 31437426718 on the versions the gating leg pins, and verified against that leg's own output: the recorded cohort.catalogue.bed12 md5 is the value CI reported as found. Exactly 16 md5s move, all in files carrying orf_class -- the six ribotish.normalised.tsv sidecars, the cohort catalogue and its consensus products, and the psite counts keyed on catalogue ORF ids. The 172 hashed entries are otherwise identical with none added or removed, and every ribotish.normalised.bed12, fastqc plot, flagstat and bigWig is unchanged. The whole-file diff is an artefact of nf-test 0.9.4 writing four-space indentation where 0.9.5 wrote two; the other four snapshots on this branch have the same shape.
suhrig
self-requested a review
August 11, 2026 07:42
suhrig
approved these changes
Aug 11, 2026
suhrig
left a comment
suhrig
left a comment
There was a problem hiding this comment.
Great enhancement! In particular, the complete linkage mapping is biologically so much more meaningful than the previous approach!
| - [#197](https://github.com/nf-core/riboseq/issues/197) - Make `--pseudo_aligner` select the tool used by `--te_quantification_method pseudo`, which always ran Salmon regardless of the setting. kallisto is now a working alternative, with `--kallisto_index`, `--kallisto_quant_fraglen`, `--kallisto_quant_fraglen_sd` and `--extra_kallisto_quant_args`. TE pseudo-alignment outputs move to `quantification/<pseudo_aligner>_te_pseudo`, unchanged for the default `salmon` ([@pinin4fjords](https://github.com/pinin4fjords)) | ||
| - [#205](https://github.com/nf-core/riboseq/pull/205) - Template update for nf-core/tools v4.0.3 ([@nf-core-bot](https://github.com/nf-core-bot), [@pinin4fjords](https://github.com/pinin4fjords)) | ||
| - [#226](https://github.com/nf-core/riboseq/pull/226) - **Breaking:** `orf_class` is now purely positional and no longer encodes ORF length. The `smORF` value is gone from every catalogue output; a short ORF keeps its positional class (`uORF`, `dORF`, `canonical_cds`, …) and is flagged by the new `is_smorf` column instead. Migrate a filter on `orf_class == "smORF"` to `is_smorf == "1"` (equivalently `0 < aa_length <= 100`, the default `--smorf_max_aa`). The peptide-collapse scope is unchanged at the default threshold ([@FelixKrueger](https://github.com/FelixKrueger)) | ||
| - [#226](https://github.com/nf-core/riboseq/pull/226) - **Breaking:** `orf_class` values change for ORFs the callers place relative to the CDS. RiboCode's and ribotricer's `Overlap_uORF`/`Overlap_dORF` were being folded into `uORF`/`dORF` by substring matching and now map to `uoORF`/`doORF`; RiboCode's and Ribo-TISH's `internal` and PRICE's `iORF` now map to `intORF` rather than `other`; PRICE's `uoORF` maps to `uoORF` rather than `uORF` and its `orphan` to `novel_u` rather than `other`. ribotricer's `internal` stays `other` because it is that tool's fall-through rather than a frame-tested call. To keep a pre-existing filter's meaning, read the old `uORF` as the new `uORF` ∪ `uoORF`, the old `dORF` as `dORF` ∪ `doORF`, and note that the old `other` no longer holds the internal ORFs. Anything consuming `orf_class` — including the ORF-level DOTSeq analysis, which matches `uORF`/`dORF` literally — sees the new vocabulary ([@FelixKrueger](https://github.com/FelixKrueger)) |
There was a problem hiding this comment.
Anything consuming
orf_class— including the ORF-level DOTSeq analysis, which matchesuORF/dORFliterally — sees the new vocabulary
Just so we are clear: This means that DOTSeq will now consider a reduced set of ORFs on this line compared to the previous approach. I'm not saying this is worse. I just want to rule out an oversight.
Contributor
Author
There was a problem hiding this comment.
Worth checking, thanks. That heatmap call is the only place orf_class reaches DOTSeq -- sorf_type appears nowhere else in the template -- so the DTE and DOU statistics and the set of ORFs they test are unchanged; what shrinks is only that figure's input, since ORFs the old substring matching folded into uORF are now uoORF and no longer satisfy sorf_type = "uORF". Happy to add uoORF and doORF as further fallbacks in a follow-up if you would rather the heatmap kept its previous coverage.
There was a problem hiding this comment.
I don't mind. One could even argue that it is better now, since those are the true upstream ORFs. Zero overlap.
| - [#197](https://github.com/nf-core/riboseq/issues/197) - Make `--pseudo_aligner` select the tool used by `--te_quantification_method pseudo`, which always ran Salmon regardless of the setting. kallisto is now a working alternative, with `--kallisto_index`, `--kallisto_quant_fraglen`, `--kallisto_quant_fraglen_sd` and `--extra_kallisto_quant_args`. TE pseudo-alignment outputs move to `quantification/<pseudo_aligner>_te_pseudo`, unchanged for the default `salmon` ([@pinin4fjords](https://github.com/pinin4fjords)) | ||
| - [#205](https://github.com/nf-core/riboseq/pull/205) - Template update for nf-core/tools v4.0.3 ([@nf-core-bot](https://github.com/nf-core-bot), [@pinin4fjords](https://github.com/pinin4fjords)) | ||
| - [#226](https://github.com/nf-core/riboseq/pull/226) - **Breaking:** `orf_class` is now purely positional and no longer encodes ORF length. The `smORF` value is gone from every catalogue output; a short ORF keeps its positional class (`uORF`, `dORF`, `canonical_cds`, …) and is flagged by the new `is_smorf` column instead. Migrate a filter on `orf_class == "smORF"` to `is_smorf == "1"` (equivalently `0 < aa_length <= 100`, the default `--smorf_max_aa`). The peptide-collapse scope is unchanged at the default threshold ([@FelixKrueger](https://github.com/FelixKrueger)) | ||
| - [#226](https://github.com/nf-core/riboseq/pull/226) - **Breaking:** `orf_class` values change for ORFs the callers place relative to the CDS. RiboCode's and ribotricer's `Overlap_uORF`/`Overlap_dORF` were being folded into `uORF`/`dORF` by substring matching and now map to `uoORF`/`doORF`; RiboCode's and Ribo-TISH's `internal` and PRICE's `iORF` now map to `intORF` rather than `other`; PRICE's `uoORF` maps to `uoORF` rather than `uORF` and its `orphan` to `novel_u` rather than `other`. ribotricer's `internal` stays `other` because it is that tool's fall-through rather than a frame-tested call. To keep a pre-existing filter's meaning, read the old `uORF` as the new `uORF` ∪ `uoORF`, the old `dORF` as `dORF` ∪ `doORF`, and note that the old `other` no longer holds the internal ORFs. Anything consuming `orf_class` — including the ORF-level DOTSeq analysis, which matches `uORF`/`dORF` literally — sees the new vocabulary ([@FelixKrueger](https://github.com/FelixKrueger)) |
Member
There was a problem hiding this comment.
Can we try to keep changelog entries a bit less verbose? Claude does tend to get carried away, they should only a be a sentence.
Contributor
Author
There was a problem hiding this comment.
Fair -- two of those had grown into full paragraphs with migration instructions inline. #227 trims all seven to a single sentence each, 440 words down to 136; the vocabulary and migration detail is already in docs/output.md and docs/usage.md, which is where it belongs.
pinin4fjords
approved these changes
Aug 11, 2026
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
3 participants
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.
orf_classcurrently mixes position with length: a short ORF is labelledsmORFwherever it sits relative to the CDS, and the CDS-overlap and internal distinctions the callers already report get folded intouORF/dORF/other. This makesorf_classpurely positional, addsuoORF,doORFandintORF, and moves length onto a separateis_smorfcolumn controlled by--smorf_max_aa(default 100, which reproduces the previoussmORFmembership exactly). It also addsorf_type_native, carrying each caller's own label so every harmonisation decision stays auditable. Breaking for anything readingorf_class; the CHANGELOG gives both migration directions.The module side is already merged — nf-core/modules#12498, #12599 and #12613 — and
modules.jsonpins them.Design notes and verification (AI-assisted)
Why class had to leave orfmerge's grouping key. Callers disagree about class for the same ORF, and Ribo-TISH structurally cannot agree:
tisType()tests only the start position, so its5'UTRcovers uORF ∪ uoORF. Keying the merge on class would emit one row per disagreeing caller. The rule this settled on: a class is safe to add only if either every caller can positively report it, or the merge is class-agnostic.Two clustering fixes came along with it. Reciprocal overlap is now measured on summed exon-block intersection rather than the outer genomic span, which for a spliced ORF is mostly intron; and clustering uses complete linkage, so a chain of partial overlaps cannot fold distinct ORFs into a single row.
Row counts move in both directions, which the CHANGELOG spells out. Down: ORFs whose callers disagreed on class now merge. Up: a short truncated CDS variant is no longer folded into its transcript's full-length CDS, and short transcript-anchored ORFs are keyed on their exact span.
Deliberately not included. No
altCDSclass (RiboCode folds variants intoannotatedand ribotricer into itsinternalfall-through, so it would fail the safety rule); ribotricer'sinternalstaysotherbecause it is that tool's terminal fall-through rather than a frame-tested call; and stop-codon keying for short transcript-anchored ORFs is documented and pinned by a test rather than fixed.Snapshots. The four ORF-catalogue snapshots were re-recorded on the versions this repo's CI pins (nf-test 0.9.4, Nextflow 25.10.4). Every md5 that moved belongs to a file carrying
orf_class; theribotish.normalised.bed12md5s did not move, which is what a classification-only change should look like.