fix(simulate): write SS sub-sort tag via SortOrder accessors (R2-HDR-01) by nh13 · Pull Request #531 · fulcrumgenomics/fgumi

nh13 · 2026-07-10T04:18:15Z

Extends #514's <sort-order>:<sub-sort> SS parity to simulate.

simulate grouped-reads/mapped-reads hand-formatted the SS tag as a bare literal; this reuses SortOrder::TemplateCoordinate.header_{so,go,ss}_tag() (the accessors #514 makes canonical) so every SS-writing site shares one mapping and simulate output carries the same unsorted:template-coordinate form.

Stacked on #514 — it depends on #514's keys.rs prefix change, so this targets nh/fix-sort-ss-header-prefix; GitHub will retarget it to main once #514 merges. Surfaced by the fgumi compare hardening validation (#530).

Summary by CodeRabbit

Bug Fixes
- Updated SAM header sort metadata to include the complete sort-order prefix in SS tags.
- Queryname headers now report values such as queryname:lexicographic and queryname:natural.
- Template-coordinate headers now report unsorted:template-coordinate.
Improvements
- Simulation outputs now consistently generate SO, GO, and SS header tags according to the selected sort order.

…/samtools parity fgbio (`SamOrder.applyTo`: `sortOrder.name() + ":" + ss`) and samtools write the `SS` header tag as `<sort-order>:<sub-sort>` — e.g. `queryname:natural`, `unsorted:template-coordinate`. fgumi wrote the bare sub-sort (`natural`, `template-coordinate`), which diverges from both parity targets: fgbio's `SamOrder.apply` strips everything up to the first colon (`s.substring(s.indexOf(':') + 1)`), so a bare `SS:natural` is not re-recognized as a queryname order. (Template-coordinate happened to round-trip by luck.) `SortOrder::header_ss_tag` now returns the `<sort-order>:<sub-sort>` form, and `create_output_header` uses it for both the queryname and template-coordinate branches. The `<sort-order>` prefix is the value written to the `SO` tag (`queryname` for queryname sub-sorts, `unsorted` for template-coordinate). No reader regression: `is_template_coordinate_sorted` already splits on the first colon and accepts both the prefixed and bare forms (mirroring fgbio), so fgumi-, fgbio-, and samtools-written headers are all accepted. There is no reader that inspects the queryname sub-sort value. R2-HDR-01.

coderabbitai · 2026-07-10T04:18:23Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 78f6139c-f2f6-4494-83d2-d018cc2861b8

📥 Commits

Reviewing files that changed from the base of the PR and between 9c24635 and 9676dc1.

📒 Files selected for processing (7)

crates/fgumi-sort/src/external.rs
crates/fgumi-sort/src/keys.rs
crates/fgumi-sort/src/lib.rs
src/lib/commands/simulate/common.rs
src/lib/commands/simulate/grouped_reads.rs
src/lib/commands/simulate/mapped_reads.rs
src/lib/commands/sort.rs

Walkthrough

Sort-order SS values now include the corresponding SO prefix. Output-header generation and simulation BAM header construction consume this format through shared logic, with tests updated for queryname and template-coordinate orders.

Changes

SAM SS header formatting

Layer / File(s)	Summary
Sort-order SS contract `crates/fgumi-sort/src/keys.rs`, `crates/fgumi-sort/src/lib.rs`, `crates/fgumi-sort/src/external.rs`, `src/lib/commands/sort.rs`	`header_ss_tag()` returns values such as `queryname:natural` and `unsorted:template-coordinate`; output-header logic and unit tests now expect the prefixed format.
Simulation header-map wiring `src/lib/commands/simulate/common.rs`, `src/lib/commands/simulate/grouped_reads.rs`, `src/lib/commands/simulate/mapped_reads.rs`	A shared helper builds SO/GO/SS tags from `SortOrder`, and both simulation commands use it for template-coordinate BAM headers.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

fulcrumgenomics/fgumi#514: Updates the same prefixed SS formatting across sort-order output and tests.
fulcrumgenomics/fgumi#519: Uses related SO/SS template-coordinate semantics in merge input validation.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the simulate SS header refactor and the use of SortOrder accessors.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch nh/simulate-ss-parity

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-07-10T04:20:39Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.11%. Comparing base (cbc7d72) to head (9676dc1).
⚠️ Report is 32 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #531      +/-   ##
==========================================
- Coverage   91.17%   91.11%   -0.07%     
==========================================
  Files          78       78              
  Lines       51916    51911       -5     
==========================================
- Hits        47333    47297      -36     
- Misses       4583     4614      +31

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

simulate grouped-reads / mapped-reads hand-formatted the `SS` sub-sort tag as a bare literal. Reuse `SortOrder::TemplateCoordinate.header_{so,go,ss}_tag()` so the emitted `@HD SS` matches the <sort-order>:<sub-sort> form this PR establishes for `sort`, keeping every SS-writing site on one canonical mapping. Stacked on #514 (depends on its keys.rs prefix).

nh13 · 2026-07-12T00:13:32Z

Note: overlap with #576 (simulate → canonical fgumi-sort)

#576 routes simulate mapped-reads/grouped-reads output through the canonical fgumi-sort engine (emit unsorted temp → RawExternalSorter(TemplateCoordinate)). That sorter writes the @HD sort tags itself — SO:unsorted GO:query SS:template-coordinate — so it may overlap or supersede this PR's SS sub-sort-tag handling in mapped_reads.rs/grouped_reads.rs. Worth reconciling before either merges (see the fuller note on #541, which is stacked on this).

…ORT3-10) The `@HD SS` sub-sort value for a lexicographic queryname sort was written as `lexicographic`, which is not the SAM-spec / samtools-ecosystem spelling (`lexicographical` — the value htsjdk/Picard emit and parsers recognize). The tag is write-only (fgumi determines sort order from `SO`/`GO` and the CLI `--order`, never by parsing an input `SS`), so this is a pure spelling correction with no functional round-trip effect. `header_ss_tag()` now returns `lexicographical`. To avoid a UX papercut — a user seeing `SS:queryname:lexicographical` in output and passing it back — `--order queryname::lexicographical` is accepted as an alias for `queryname::lexicographic`. The CLI vocabulary and the type's `Display` keep the shorter `lexicographic` spelling. Composes with the separate `<SO>:<sub-sort>` prefix fix (SORT3-01, #514/#531 in lib.rs): after both, the header reads `SS:queryname:lexicographical`. Verified: `fgumi sort --order queryname` emits `SS:lexicographical` on this branch.

nh13 · 2026-07-16T05:38:30Z

@coderabbitai review

coderabbitai · 2026-07-16T05:38:36Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

nh13 · 2026-07-16T06:37:44Z

@coderabbitai review

coderabbitai · 2026-07-16T06:37:51Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

nh13 · 2026-07-16T06:38:42Z

@coderabbitai review

coderabbitai · 2026-07-16T06:38:47Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…ORT3-10) The `@HD SS` sub-sort value for a lexicographic queryname sort was written as `lexicographic`, which is not the SAM-spec / samtools-ecosystem spelling (`lexicographical` — the value htsjdk/Picard emit and parsers recognize). The tag is write-only (fgumi determines sort order from `SO`/`GO` and the CLI `--order`, never by parsing an input `SS`), so this is a pure spelling correction with no functional round-trip effect. `header_ss_tag()` now returns `lexicographical`. To avoid a UX papercut — a user seeing `SS:queryname:lexicographical` in output and passing it back — `--order queryname::lexicographical` is accepted as an alias for `queryname::lexicographic`. The CLI vocabulary and the type's `Display` keep the shorter `lexicographic` spelling. Composes with the separate `<SO>:<sub-sort>` prefix fix (SORT3-01, #514/#531 in lib.rs): after both, the header reads `SS:queryname:lexicographical`. Verified: `fgumi sort --order queryname` emits `SS:lexicographical` on this branch.

…ORT3-10) (#567) The `@HD SS` sub-sort value for a lexicographic queryname sort was written as `lexicographic`, which is not the SAM-spec / samtools-ecosystem spelling (`lexicographical` — the value htsjdk/Picard emit and parsers recognize). The tag is write-only (fgumi determines sort order from `SO`/`GO` and the CLI `--order`, never by parsing an input `SS`), so this is a pure spelling correction with no functional round-trip effect. `header_ss_tag()` now returns `lexicographical`. To avoid a UX papercut — a user seeing `SS:queryname:lexicographical` in output and passing it back — `--order queryname::lexicographical` is accepted as an alias for `queryname::lexicographic`. The CLI vocabulary and the type's `Display` keep the shorter `lexicographic` spelling. Composes with the separate `<SO>:<sub-sort>` prefix fix (SORT3-01, #514/#531 in lib.rs): after both, the header reads `SS:queryname:lexicographical`. Verified: `fgumi sort --order queryname` emits `SS:lexicographical` on this branch.

nh13 temporarily deployed to github-actions July 10, 2026 04:18 — with GitHub Actions Inactive

nh13 force-pushed the nh/simulate-ss-parity branch from 8dcf85f to 9676dc1 Compare July 10, 2026 06:10

nh13 temporarily deployed to github-actions July 10, 2026 06:11 — with GitHub Actions Inactive

This was referenced Jul 10, 2026

feat(simulate): body error injection, template-coordinate sort fix, correctness guards (SIMU3-01/03/04/05/07) #541

Merged

fix(sort): emit spec sub-sort spelling lexicographical in @HD SS (SORT3-10) #567

Merged

Base automatically changed from nh/fix-sort-ss-header-prefix to main July 13, 2026 03:13

nh13 merged commit 1ef9481 into main Jul 16, 2026
11 checks passed

nh13 deleted the nh/simulate-ss-parity branch July 16, 2026 15:00

nh13 mentioned this pull request Jul 16, 2026

chore: release v0.5.0 #454

Merged

Navigation Menu

Repository navigation

fix(simulate): write SS sub-sort tag via SortOrder accessors (R2-HDR-01) - #531

fix(simulate): write SS sub-sort tag via SortOrder accessors (R2-HDR-01)#531
nh13 merged 2 commits into
mainfrom
nh/simulate-ss-parity

nh13 commented Jul 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 10, 2026 •

edited

Loading

Uh oh!

nh13 commented Jul 12, 2026

Uh oh!

nh13 commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

Uh oh!

nh13 commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

Uh oh!

nh13 commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

nh13 commented Jul 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

codecov Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nh13 commented Jul 12, 2026

Note: overlap with #576 (simulate → canonical fgumi-sort)

Uh oh!

nh13 commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nh13 commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nh13 commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

This branch was previously deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

nh13 commented Jul 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

codecov Bot commented Jul 10, 2026 •

edited

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading