Repository navigation
feat(summary): ClinVar, PGx, ACMG SF and two-hit sections in Rust - #404
Merged
Merged
Conversation
og dev build cn-intervals, callability and sv-rule-regions read the pinned downloads from a folder and write bin/cn_loci.tsv's interval rows, assets/callability_genes.tsv and assets/sv_rule_regions.tsv, byte for byte as the Python builders did. The GTF is read as bytes with a hand scanner of its attributes, the cut sets are merged once per chromosome and searched by binary search, and og never downloads. The SV rule builder now keeps the first GENCODE record per HGNC id and skips _PAR_Y copies, as the callability builder does; the committed table does not change. Bead: pgp-bs9u.35
build-callability.sh and build-sv-rule-regions.sh keep their downloads and hand the folder to og dev build; their Python heredocs are gone. build-cn-intervals.py becomes build-cn-intervals.sh with the same --list, --cache and --check. The three share scripts/ci/lib/fetch-pinned.sh: a cached file with the right sha256 is read in place, a 403 or 404 fails at once, anything else is retried for about five minutes. The marker line of bin/cn_loci.tsv names the new script; no row changes. The cn-intervals case, the e2e data fill and the converter checks of tests/test_copy_number.py call the new script and og. Bead: pgp-bs9u.35
Red-first control, reverted in the next commit: callability keeps the _PAR_Y copies, the SV rule builder keeps the last record per HGNC id, and each cn-intervals piece ends one base short. Bead: pgp-bs9u.35
… --only b) ClinVar, the carrier list, PharmCAT, CPIC, pypgx, Cyrius, CPSR, the clinical filter with its ACMG SF tier, slivar and the cyp2d6 block, each input read once, beside the Python collector. No caller changes. Bead: pgp-bs9u.42
The clinical VCF, slivar's VCF and the hits were read whole before being split; now one pass streams each, the hits are no longer copied for the ClinVar section, and the summary's own data is freed before the JSON text is made. A table cut short reads as Python's EOFError. Bead: pgp-bs9u.42
The note now says "line L column C (char N)" in code points after universal newlines, and names a UTF-8 BOM, as json.load does. Bead: pgp-bs9u.42
…thon Ten cases of og summary collect --only b: every input in both layouts, failed and stale steps, unreadable inputs, a VEP output with and without its index, phased deletions in trans and in cis, and no assets. The oracle stages the helper modules the collector imports from the pinned commit instead of stubs. cb-notgz holds the one deliberate fix. Bead: pgp-bs9u.42
…rate map Bead: pgp-bs9u.42
Plant: the deletion's read-backed phase is ignored, so a variant in cis with a deletion counts as a second hit and a trans pair is only possibly biallelic. The two_hits unit tests and the cb-full parity case must fail. Bead: pgp-bs9u.42
|
Warning Review limit reachedYou'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 6 minutes. View limit details
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. |
…go red" The plant turned five builder tests red in Guard's rust job (run 43417543846); back to the right rules. Bead: pgp-bs9u.35
… failed fetch_pinned downloads to a mktemp file beside the target, so two runs on one cache never write the same partial file, and gives curl a 30 s connect timeout and a 1 KB/s for 60 s floor, so a stalled server reaches the retry or the error instead of holding the build. Bead: pgp-bs9u.35
Revert the acmg-biallelic-ignores-phase control. Keep cis variants out of the two-hit count and classify a phased trans pair as biallelic. Bead: pgp-bs9u.42
Keep the invented data rows separate from their column labels, as the other generated tables do. This preserves every fixture byte and keeps the personal-data scanner enabled without adding an exemption. Bead: pgp-bs9u.42
# Conflicts: # CHANGELOG.md
Collaborator
Author
|
The independent review's two input bugs are fixed in 1a883cd. The final head is 990999c after one conflict-required merge of main.
The group-b pattern check found no further unchecked fixed-offset string slice or TSV-header split. The common line iterator preserves Red evidence from the pre-fix collector on this box, using Rust 1.93.0 and the new synthetic inputs: Green on the final head: 64 crate tests pass. All 188 comparisons across 47 summary cases agree with the oracle. All 44 comparator controls pass. The harness reports all 15 controls passing, with its Linux shadow check deferred to CI. Formatting, clippy with The earlier phase-control red CI run and its restored green run remain linked in the PR. The planted fault is absent from the final head. No e2e case was added or changed. The new collection mode is covered by the subtree cases. E2E and landing remain with the landing train. |
# Conflicts: # CHANGELOG.md # rust/crates/og-dev/src/lib.rs # rust/og/src/main.rs
…list zram0 Some runner images list /dev/zram0 as a disk of 0 bytes with no file system. The spare-disk step took it for the blank disk, and mkfs.ext4 stopped the leg before any case ran. The step now skips zram, ram and loop devices and any disk of 0 bytes.
…-r2b # Conflicts: # CHANGELOG.md
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
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.
The summary collector still needs Python for the clinical sections. This adds
og summary collect --only bso those sections can be checked before the collector switches.It reads ClinVar, the carrier list, PharmCAT, CPIC, pypgx, Cyrius, CPSR, clinical variants and slivar, plus the CYP2D6 block. The clinical section includes ACMG SF v3.3 and deletion-plus-variant two-hit rules. ClinVar stars use
og-clinvar, indexed VEP lookups use the shared tabix reader, and CYP2D6 usesog-steps::pgx_rules.The folder index is reused. ClinVar hits feed multiple sections from one read. Clinical VCF records stream through one pass for counts and HIGH-impact candidates. Section errors stay inside their sections.
Decision: collect clinical subtrees beside the existing collector so incomplete summaries cannot replace production output. Caller switching and Python deletion follow once all section groups are complete. Production reports continue using the existing collector. Output names and formats stay the same.
Deliberate differences, also listed in CHANGELOG.md:
.vcf.gzsuffix is readable. The Python collector marks it unreadable. Thecb-notgzgolden pins this change.Shared files changed for this group: rust/Cargo.lock records two existing workspace dependencies. rust/og/src/main.rs lists
--only b. docs/rust.md and rust/README.md describe it. The group's rows and generator extend tests/parity/cases.tsv. Its oracle wrapper loads the pinned helper modules used by the clinical sections.The expanded package scope also covers these parity files: tests/parity/gen/summary-clinical.sh, tests/parity/inputs/summary-collect/oracle.sh, cb_vep.vcf.gz, cb_vep.vcf.gz.tbi and the cb-notgz golden. They provide synthetic input, the pinned oracle and the deliberate output difference.
The independent review found two input bugs. Quoted headers in the SMN, interval and dosage tables discarded deletion evidence. A non-ASCII consensus contig could stop the collector before it wrote a summary. The fixes parse the first TSV record with
pycompat::csv::Readerand check the ASCII chromosome prefix on bytes. Header reads stop after the first record. No output name or format changes.The pattern check found no other unchecked fixed-offset string slice or TSV-header split in group b. The other tab splits read VCF records. The shared line iterator keeps
rows()behavior unchanged. Both readers remain inside group b, so R2c's sections keep their readers.How it was checked
990999c8, the phase fault was reapplied locally and still failed the same three tests: 61 passed and 3 failed. After restoring the file, all 64 tests passed. The merged container-helper static check also passed.cargo +1.93.0 test --locked -p og-summary --no-fail-fast: 61 passed and 3 failed. All four new parity cases failed against the pre-fix collector. The restored collector passes 64 tests and all 188 comparisons across 47 cases. The local harness reports all 15 controls passing. Its Linux shadow check remains CI-only.990999c8b173a54c95391d9a455196edd1ead034. Both changelog entries were retained. The 64 tests and all local parity checks passed again on this head.8fb444c6plantsacmg-biallelic-ignores-phase. Guard's cargo test step failed three two-hit tests: cis exclusion, trans classification and best-pair selection. The remaining red-head jobs were cancelled. Commitb5d1a1f1restores the rule.cargo test --locked -p og-summary --no-fail-fast: 64 passed after the review fixes. Formatting, clippy with-D warnings, and docs withRUSTDOCFLAGS=-D warningspass on the pinned Rust 1.93.0 toolchain.ORACLE_RUNNER=local scripts/ci/oracle-diff.sh summary-collect: 47 cases, 188 items agree, including the deliberate golden. All 15 harness self-tests pass.c3bd0a264e642be978d8310c7ec539fedde46c87: Guard with ci-ok, Rust, Lint, Nextflow, Docs, Container Test and Personal data. Release, Renovate and Freshness also passed.Synthetic timing, three runs on the same Apple Silicon box,
/usr/bin/time -l(RSS in decimal MB):The large-hit stress case uses more memory in Rust because the summary is built as a JSON tree. This is a measured limit, and this PR makes no whole-pipeline speed claim. Only synthetic inputs were measured.
CodeRabbit returned a rate-limit notice. No review has arrived. Another PR received a review at 13:24 UTC, so the shared 55-minute queue prevents a new request. The independent review findings are fixed. The review response and local red evidence record each fix.
Current head:
990999c8b173a54c95391d9a455196edd1ead034. Fast checks are running: Guard, Rust, Nextflow and Docs. Lint, Container Test and Personal data passed.Bead implemented: pgp-bs9u.42.