Skip to content

refactor(traits): build the traits table in Rust and drop the Python it replaces - #408

Merged
GeiserX merged 9 commits into
GeiserX:mainfrom
geiresx:pgp/rust-c2b
Nov 13, 2026
Merged

refactor(traits): build the traits table in Rust and drop the Python it replaces#408
GeiserX merged 9 commits into
GeiserX:mainfrom
geiresx:pgp/rust-c2b

Conversation

@geiresx

@geiresx geiresx commented Nov 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Step 42 still depended on Python to read traits and haplotype rules. An unused Python copy-number engine also remained beside the Rust engine.

The step now calls og traits table, phase-sites and check. The replaced evaluator and copy-number engine are removed. Report metadata remains for its importers. Output columns, names and readings stay the same. Readers retain only catalog sites. Both outputs are staged before publication. Staging failures and directory destinations preserve both previous tables. The two final renames are separate, so a second-rename failure can leave the first table updated. Write errors name the path and exit cleanly.

The catalog stays plain TSV. Quotes remain literal, so quoted headers, quoted fields and tabs inside quotes cannot bypass validation. The report collector still reads the output with QUOTE_NONE. The six fixed reference labels, flipped-allele controls and per-rule HTML assertions run on og-generated tables. Test-side checks validate the retained marker metadata and its swap, complement, off-copy and wrong-context controls.

How it was checked

  • Rust 1.93.0: all 101 og-steps tests pass. Rustfmt, clippy with warnings denied, and cargo doc pass for the changed crate. The initial port also passed 739 workspace tests.
  • Local oracle after these fixes: all 409 comparisons in 102 traits/CN cases agree. The parity harness passes its 15 controls and the comparator passes its 44 controls. All eight traits, catalog, copy-number, indel-rule, summary and report test scripts pass with OG_REQUIRED=1.
  • The synthetic demo build passes. Strict docs build and site-check pass on 96 pages.
  • rust-c2b-traits passes all 12 assertions locally. A fresh mount probe confirms Docker cannot read the host scratch file. traits-panel-1, 42-slivar-31 and followups-clinvar-traits-1-rules-html need fork CI.
  • Red-first evidence: new tests fail on the unfixed parser and directory output. Both WRONG_REFERENCE catalog mutations and the off-copy marker metadata mutation fail their specific restored assertions. Guard repeats them and requires exactly one expected assertion failure per mutation.
  • The original Guard controls still catch all three faults: reversed diplotype order, a removed REF check and missing phase rows. The first planted probe stopped at formatting. Commit ab5088dc reverted it. The subsequent Guard run ran the controls with the source restored afterwards.
  • Final head: d0c12a52666fe22a01112b095ab9df2d2f129c77. Upstream fast checks are queued or running on the checks page. Guard and Rust must pass before landing.

Fork E2E scheduling and landing remain with the maintainer. The three container-dependent cases remain unverified for this port.

Five runs per input on macOS ARM64, Python 3.14 versus Rust release. Wall time is the median. Memory is the maximum peak. Both output files compare byte for byte.

Synthetic input Python wall / memory Rust wall / memory
36 catalog records 0.04 s / 20.0 MB 0.02 s / 3.8 MB
Same records plus 200,000 unrelated sites 0.33 s / 137.2 MB 0.03 s / 3.9 MB
Files needed beyond the initial lane list
  • rust/og/src/main.rs: expose the three verbs.
  • tests/demo/make_demo_sample.py, tests/e2e/genotype-rules-2-g2.sh, tests/e2e/report-followups-1-sections.sh: replace calls to the deleted evaluator.
  • tests/test_collect_summary.py, tests/test_genotype_rules_indels.py, tests/test_render_report.py: preserve report assertions through og. Shards without a binary use the pinned historical oracle.
  • .github/workflows/guard.yml: run CLI tests and named fault controls. tests/e2e/rust-c2b-traits.sh: test the Rust commands. scripts/ci/e2e-case-paths.tsv, scripts/ci/e2e-case-times.tsv: register coverage and remove the deleted path.
  • tests/parity/cases.tsv, tests/parity/oracle.ref, tests/parity/gen/traits.sh: compare the port with its pinned oracle. tests/parity/remnants.tsv: list retained metadata. tests/parity/README.md, tests/parity/evidence/rust-c2b-controls.txt: explain coverage and retain failing-control evidence.
  • docs/rust.md, rust/README.md: document the commands. docs/cn-loci.md: remove the claim that the deleted engine remains.

Bead: pgp-bs9u.25

@geiresx
geiresx requested a review from GeiserX as a code owner November 13, 2026 14:11
@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •
edited
Loading

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 21 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 5aa373f and f8df002.

Files ignored due to path filters: 5

  • scripts/ci/e2e-case-paths.tsv is excluded by !**/*.tsv
  • scripts/ci/e2e-case-times.tsv is excluded by !**/*.tsv
  • tests/parity/cases.tsv is excluded by !**/*.tsv
  • tests/parity/gen/traits.sh is excluded by !**/gen/**
  • tests/parity/remnants.tsv is excluded by !**/*.tsv
Files selected for processing: 28

  • .github/workflows/guard.yml
  • CHANGELOG.md
  • bin/copy_number.py
  • bin/traits_report.py
  • docs/42-traits.md
  • docs/cn-loci.md
  • docs/rust.md
  • modules/local/traits/main.nf
  • rust/README.md
  • rust/crates/og-steps/src/lib.rs
  • rust/crates/og-steps/src/traits.rs
  • rust/og/src/main.rs
  • scripts/42-traits.sh
  • tests/demo/make_demo_sample.py
  • tests/e2e/genotype-rules-2-g2.sh
  • tests/e2e/report-followups-1-sections.sh
  • tests/e2e/rust-c2b-traits.sh
  • tests/parity/README.md
  • tests/parity/evidence/rust-c2b-controls.txt
  • tests/parity/oracle.ref
  • tests/test_collect_summary.py
  • tests/test_copy_number.py
  • tests/test_genotype_rules_indels.py
  • tests/test_hba_copy_number.py
  • tests/test_lpa_kiv2.py
  • tests/test_render_report.py
  • tests/test_traits_report.py
  • tests/test_traits_table.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@geiresx geiresx mentioned this pull request Nov 13, 2026
Closed
@GeiserX
test(traits): restore evaluation after planted controls
ab5088d 
Reverts 339b647.

Bead: pgp-bs9u.25
@GeiserX
fix(traits): keep shared sites independent and stage both outputs
0e0ba11 
Exercise the three named fault controls in Guard before testing the restored source. Retain the local red log as reviewable evidence.

Bead: pgp-bs9u.25
@GeiserX
docs(traits): describe the removed copy-number test oracle
853dda6 
Bead: pgp-bs9u.25
@GeiserX
test(traits): restore independent expectations and output refusal con…
b7be249 
…trols
@GeiserX
fix(traits): preserve plain TSV and reject directory outputs before p…
d0c12a5 
…ublication
@GeiserX
Merge commit '5aa373f2534a487df7d2d4cc44febc17f2a7b800' into pgp/rust…
f8df002 
…-c2b

# Conflicts:
#	CHANGELOG.md
#	bin/copy_number.py
#	docs/rust.md
#	scripts/ci/e2e-case-paths.tsv
#	tests/parity/oracle.ref
@GeiserX
GeiserX merged commit 11cf6c9 into GeiserX:main Nov 13, 2026
46 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GeiserX GeiserX Awaiting requested review from GeiserX GeiserX is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@geiresx @GeiserX