Skip to content

fix(vep): translate with the transcript codon table (MT table 2) - #274

Merged
mwiewior merged 5 commits into
masterfrom
fix/mt-codon-table-200
Oct 19, 2026
Merged

mwiewior merged 5 commits into
masterfrom
fix/mt-codon-table-200

Conversation

@mwiewior

@mwiewior mwiewior commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Engine side of biodatageeks/vepyr#200. Based on functions master after #273 (vepyr#199) merged as 4704ff9; this PR now targets master. Reader fix: biodatageeks/datafusion-bio-formats#262, pinned here by rev. vepyr PR: biodatageeks/vepyr#206.

Problem

VEP translates a transcript with BaseTranscriptVariation::_codon_table, which is table 2 (vertebrate mitochondrial) for every human MT transcript. The engine only knew the standard code. On chrMT, ATA read as Ile instead of Met, AGA/AGG as Arg instead of stop, and TGA as stop instead of Trp. Amino_acids, HGVSp, Consequence, IMPACT, SIFT and PolyPhen followed: 294 of 3,134 ClinVar chrMT records differed.

Where VEP uses the table (ensembl-variation 2fb834b = 116.0, db7394a = 116.2; the modules are identical)

  • With the transcript's table: TranscriptVariationAllele::peptide() calls translate(undef, undef, undef, $codon_table) (TVA.pm:692-788). That feeds Amino_acids, every peptide-based predicate, SNV/in-frame HGVSp peptides and the SIFT/PolyPhen key. Also with the table: the reference protein _peptide (the cached peptide, table 2) and the codon now at the old stop position in _ins_del_stop_altered/_cil (VariationEffect.pm:1472-1480, 1527-1535).
  • Table 1, even on MT: _get_fs_peptides, _get_del_peptides, _stop_loss_extra_AA and _check_for_peptide_duplication call BioPerl's translate() with no table. VEP's own output confirms this: MT:10121 p.Trp23LeufsTer44 is the table-1 count (table 2 would give 10), and MT:14488 fsTer14 is too (table 2 would give 23).
  • Literal "ATG": start_retained_variant / _snp_start_altered compare the mutated codon with the string (VariationEffect.pm:1022), so table 2's extra start codons never matter.

Change

  • CodonTable (Standard / VertebrateMitochondrial) lives on TranscriptFeature. It is resolved at load from the cache's codon_table column. A null on a mitochondrial contig resolves to table 2, so caches converted before perf(vep): scale the LazyFrame run pool with workers (output budget, stateful run floor) #262 (all published ones) work unchanged. This matches VEP's GFF path, which assigns table 2 to MT by name.
  • The table-aware translators (translate_codon_in, translate_codon_bytes_in, translate_protein_from_cds_in, is_stop_codon_in) delegate to the existing table-1 functions, which are unchanged.
  • Threaded into the call sites that mirror peptide(), the reference protein and _ins_del_stop_altered. The HGVSp alternate-protein translations stay on table 1, and translate_protein_from_cds documents why.
  • start_retained now checks for a literal ATG (mutated_cds_starts_with_atg) instead of "the new first residue is Met". The two are identical under table 1, but under table 2 ATA is Met too: MT:8529 G>A (ATP6 ATG>ATA) is synonymous_variant in VEP.
  • is_mitochondrial_chrom in annotate_provider.rs now uses the shared is_mitochondrial_contig.
  • The bio-formats pins (all 7 entries) move to fix(ensembl-cache): read codon_table from _variation_effect_feature_cache datafusion-bio-formats#262's merge commit abac8df (91d6679). Its tree is identical to the PR head 397f15d they named before.

Tests

Written first:

  • mt_ata_start_codon_is_met_under_table_2 (MT:3307 M/L start_lost; ATA>ATT M/I)
  • mt_aga_is_a_stop_under_table_2 (MT:3308 M/* stop_gained&start_lost; an AGA terminal stop to AGC gives */S stop_lost)
  • mt_tga_is_trp_under_table_2 (MT:3669 W synonymous)
  • mt_atg_to_ata_start_is_synonymous_not_start_retained (MT:8529)
  • vertebrate_mitochondrial_table_differs_from_standard_only_on_four_codons (all 64 codons)
  • cached_transcript_codon_table_falls_back_to_table_2_on_mt

With the translators forced to table 1, the four table-2 classification tests fail, and the MT:8529 test failed before its fix. cargo test -p datafusion-bio-function-vep (1,181 unit tests plus the integration tests), cargo clippy --all-targets --all-features -- -D warnings and cargo fmt --all -- --check pass.

Verification (vepyr built against the previous head)

The results below are pre-rebase evidence. On 3f684d0 (the head before the re-pin to abac8df, with the same code), build/test CI passed and Claude's review found no blocking issue; formats#262 has merged, and the final vepyr gates will run once #274 merges and #206 is re-pinned.

  • ClinVar chrMT codon_table fixture (12 records): empty diff against VEP 116.0 and VEP 116.2. This holds on the published cache (null column, fallback) and on chrMT rebuilt from the raw cache with perf(vep): scale the LazyFrame run pool with workers (output budget, stateful run floor) #262 (codon_table = 2 on all 74 transcripts).
  • Full ClinVar chrMT (3,134 records) against VEP 116.0 and 116.2: 3,108 identical. The 26 remaining are out of scope: 20 large deletions (vepyr#202), 3 TRNT HGVS-flank records (vepyr#201), MT:4472 ND2 seq edit (vepyr#205), MT:9204 (vepyr#203 plus a stop-overlap consequence) and MT:9203 (HGVSp delextTer?). The published and rebuilt caches give byte-identical output.
  • Nuclear transcripts keep table 1 through unchanged code paths. The HG002 strict-md5 gate result will be posted on the vepyr PR.

🤖 Generated with Claude Code

@mwiewior

Copy link
Copy Markdown
Contributor Author

@codex review

@mwiewior

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T15:55:04.899639Z 3f684d0 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b2f4b511c5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +10530 to +10535
fn is_stop_codon_in(table: CodonTable, allele: &str) -> bool {
match table {
CodonTable::Standard => is_stop_codon(allele),
CodonTable::VertebrateMitochondrial => matches!(
allele.to_ascii_uppercase().as_str(),
"TAA" | "TAG" | "AGA" | "AGG" | "*"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use the transcript table in fallback stop detection

When a mitochondrial transcript has no usable translation/CDS data, the code falls back to the allele-pattern checks in classify_coding and add_start_stop_heuristic_terms, but those still call the table-1-only is_stop_codon. Consequently, a positive-strand MT codon-sized substitution such as CGA>AGA misses stop_gained even though AGA is a stop in table 2, while TGA>TAA can be treated as stop-retained even though TGA encodes Trp. Route these fallback checks through the transcript's codon_table as well.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in c684d8e. The allele-pattern stop checks in the no-CDS fallback, in classify_coding and add_start_stop_heuristic_terms, now use is_stop_codon_in(tx.codon_table, …).

New test: stop_heuristic_uses_the_transcript_codon_table. MT AGA>TAA at the stop codon gives stop_retained_variant under table 2 and not under table 1. It fails with the old calls.

Start checks in the fallback still compare the literal ATG, as VEP does. The commit also moves the bio-formats pins to #262's follow-up (397f15d). vepyr#206 is re-pinned. Its chrMT output is byte-identical to the previous head, and the regression test passes.

mwiewior added a commit that referenced this pull request Oct 18, 2026
…7f15d

Review follow-up on #274: classify_coding's allele-pattern stop check and
add_start_stop_heuristic_terms, used when a transcript has no usable CDS,
called the table-1 is_stop_codon, so on MT AGA was not a stop and TGA
was. Route them through the transcript's codon table. Moves the
bio-formats pins to #262's review follow-up (397f15d).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@mwiewior

Copy link
Copy Markdown
Contributor Author

@codex review

@mwiewior

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Claude finished @mwiewior's task in 23s —— View job


Review of PR #274: MT codon table 2

The change looks correct and I found nothing blocking. I read the diff against fix/csq-transcript-order-199 and did not build or run the tests.

What I checked

  • Table-aware translators. translate_codon_in, translate_codon_bytes_in and is_stop_codon_in override exactly TGA→W, ATA→M and AGA/AGG→* for table 2. Everything else goes to the unchanged table-1 functions.
  • Table 1 still used where VEP uses it. translate_protein_from_cds is now a table-1 wrapper, and its doc comment says why. The frameshift, deletion and stop-loss HGVSp alternate proteins stay on it, which matches the VEP behaviour you documented (MT:10121 and MT:14488).
  • Stop checks. After c684d8e, the fallback and heuristic paths no longer call plain is_stop_codon. In transcript_consequence.rs it now appears only in the definition and as the Standard arm of is_stop_codon_in. That addresses the Codex P2 comment.
  • start_retained. The literal-ATG check (mutated_cds_starts_with_atg) is applied on both the SNV/deletion path and the insertion path. It is identical to the old check under table 1 and fixes MT:8529 under table 2.
  • Null codon_table on MT. for_cached_transcript resolves a null on M, MT, chrM or chrMT to table 2. It is used in both transcript loaders, and codon_table was added to both SELECT lists (TRANSCRIPT_CONTEXT_COLUMNS and the other one). Replacing is_mitochondrial_chrom with the shared is_mitochondrial_contig is a clean dedupe.

Minor, non-blocking

  1. translate_codon_in matches "TGA", "ATA" and "AGA"/"AGG" case-sensitively, the same as translate_codon. That is fine as long as callers uppercase first. peptide_from_codon_allele_in does, but local_peptide_from_codon_window and translate_protein_from_cds_in pass the CDS bytes through as-is. If a cache ever stored lowercase CDS, table 2 would fall through to table 1 silently, the same way table 1 would return None. A debug_assert or a note in the doc would be enough.
  2. from_ncbi_id maps any unknown id to Standard silently. That is reasonable for human caches. A one-time warn for ids other than 1 and 2 would make a future cache with other tables easier to notice.
  3. pep_allele_string_from_codon_allele_string and frameshift_deletion_partial_stop_lost_from_codon_allele_string are now #[cfg(test)] table-1 wrappers. That is fine, but they are only used by existing tests, so those tests don't exercise table 2. The new MT tests cover that path instead.
  4. The is_stop_codon_in table-2 arm hard-codes TAA|TAG|AGA|AGG|*. This is correct for table 2, and the all-64-codon test guards the translator. A small test for is_stop_codon_in over every codon would pin it against translate_codon_in == '*'.

Tests and verification
The test list covers the table-2 cases: ATA start, AGA stop, TGA Trp, the MT:8529 start-retained case, the 64-codon diff, the null-on-MT fallback and the stop-heuristic regression. That is good coverage. The verification numbers in the description (3,108 of 3,134 identical, with the remaining 26 attributed to other issues) are consistent with a scoped fix.

I have no changes to request.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c684d8ed75

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

translate_codon_bytes(codon)
}

fn is_stop_codon_in(table: CodonTable, allele: &str) -> bool {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Route the CIL stop fallback through the transcript table

For VEP 116 terminal indels whose alternate peptide is absent or contains X, vep116_stop_retained_without_alt_peptide still checks the reconstructed terminal codon with the table-1-only is_stop_codon at line 4524 rather than this table-aware helper. On an MT transcript, for example, deleting the final base from a CDS ending AGA while an A shifts in from the 3′ UTR leaves AGA in the original stop slot, but this path returns false and omits stop_retained_variant; the inverse error occurs when the shifted codon is TGA. Pass tx.codon_table through this CIL fallback as well.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 5c10a90. vep116_stop_retained_without_alt_peptide (the engine copy of _ins_del_stop_altered_cil) now checks the stop-slot codon with is_stop_codon_in(tx.codon_table, …).

New test: vep116_cil_stop_slot_uses_the_transcript_codon_table. An MT CDS ending AGA with a 3′ UTR A: deleting the last base keeps AGA in the slot, which is stop_retained under table 2 and not under table 1. It fails with the old call.

In the engine, is_stop_codon is now only called through is_stop_codon_in. vepyr#206 is re-pinned (75e86cf), and its chrMT output is unchanged.

mwiewior added a commit that referenced this pull request Oct 18, 2026
Review follow-up on #274: vep116_stop_retained_without_alt_peptide
(VEP's _ins_del_stop_altered_cil) checked the codon left in the original
stop slot with the table-1 is_stop_codon. Use the transcript's table, as
VEP's _codon_table does: on MT a CDS ending in AGA whose final base is
deleted, with an A shifting in from the 3' UTR, keeps AGA in the slot and
is stop_retained.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@mwiewior

Copy link
Copy Markdown
Contributor Author

@codex review

@mwiewior

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Claude finished @mwiewior's task in 26s —— View job


Review of PR #274: MT codon table 2

I found nothing blocking. I read the diff against fix/csq-transcript-order-199 and spot-checked the remaining call sites. I did not build or run the tests.

Checked

  • All four commits are in scope (a325a4d, b2f4b51, c684d8e, 5c10a90). The earlier Codex P2 comments are addressed: the no-CDS fallback, classify_coding, add_start_stop_heuristic_terms and the VEP 116 CIL stop-slot fallback all go through is_stop_codon_in(tx.codon_table, …).
  • In transcript_consequence.rs, plain is_stop_codon is now called only from the Standard arm of is_stop_codon_in.
  • Table 2 overrides exactly TGA→W, ATA→M and AGA/AGG→*. Everything else falls through to the unchanged table-1 functions, so nuclear transcripts are unaffected.
  • for_cached_transcript resolves a null codon_table on an MT contig to table 2. Caches published before perf(vep): scale the LazyFrame run pool with workers (output budget, stateful run floor) #262 therefore keep working.
  • The remaining translate_protein_from_cds (table-1) callers are the HGVSp alternate-protein paths (around lines 5119, 5220, 5384, 5614, 6006 and 6108). That matches the documented VEP behaviour (_get_fs_peptides, _get_del_peptides, _stop_loss_extra_AA), confirmed by MT:10121 and MT:14488.

Minor, non-blocking

  1. Mixed tables at line 5384. old_aas comes from reference_aas_for_consequence (table-aware), while new_aas is table 1. If this feeds a ref/alt comparison for an HGVS shape such as delins or dup detection, a codon like ATA or TGA outside the edited region would differ between the two sides. If the comparison only reads the edited window, this is harmless. The 3,108/3,134 chrMT parity suggests it is fine, but a short comment at that site would help the next reader.
  2. Case sensitivity. translate_codon_in and translate_codon_bytes_in match uppercase only, the same as the table-1 functions. is_stop_codon_in uppercases its input. This is only a problem if a cache ever held lowercase CDS.
  3. Unknown table ids. from_ncbi_id silently maps any id other than 2 to Standard. That is fine for human caches. A one-time warning would help if other tables are ever added.
  4. Extra test. A test asserting is_stop_codon_in(t, c) == (translate_codon_in(t, c) == Some('*')) for all 64 codons under both tables would pin the hard-coded TAA|TAG|AGA|AGG list. The existing 64-codon test only covers the translator.

Tests
Coverage is good: ATA start, AGA stop, TGA as Trp, MT:8529 start-retained, the 64-codon diff, the null-on-MT fallback, and the stop-heuristic and CIL stop-slot regressions. Each of the last two fails with the old call.

No changes requested.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 5c10a904de

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

mwiewior and others added 4 commits October 18, 2026 17:45
VEP translates a transcript with BaseTranscriptVariation::_codon_table,
which is 2 (vertebrate mitochondrial) for every human MT transcript. The
engine only knew the standard code, so on chrMT ATA read as Ile instead
of Met, AGA/AGG as Arg instead of stop and TGA as stop instead of Trp,
and Amino_acids, HGVSp, Consequence, IMPACT, SIFT and PolyPhen followed
(294 of 3,134 ClinVar chrMT records, biodatageeks/vepyr#200).

TranscriptFeature now carries a CodonTable, read from the cache's
codon_table column; a null on a mitochondrial contig resolves to table 2,
so caches converted before the bio-formats reader fix work unchanged.
The table is used exactly where VEP uses it:

- with the transcript's table: everything VEP derives from
  TranscriptVariationAllele::peptide() (Amino_acids, the consequence
  predicates, SNV and in-frame HGVSp peptides, the SIFT/PolyPhen key),
  the reference protein, and the codon at the old stop position
  (_ins_del_stop_altered);
- table 1, as VEP does even on MT because BioPerl's translate() gets no
  table there: the alternate proteins behind frameshift, deletion and
  stop-loss HGVSp (_get_fs_peptides, _get_del_peptides,
  _stop_loss_extra_AA, _check_for_peptide_duplication);
- literal ATG for start_retained, as VEP compares the string.

Also moves the bio-formats pins to the reader fix (3fe9f8b), which reads
codon_table from _variation_effect_feature_cache.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
VEP's _snp_start_altered / start_retained_variant compare the mutated
start codon with the string "ATG" (VariationEffect.pm:1022). The engine
checked whether the new first residue is Met, which is the same thing
under the standard code but not under table 2, where ATA is Met too:
MT:8529 G>A (ATP6 ATG>ATA) came out start_retained_variant where VEP
writes synonymous_variant, p.Met1=. Check the codon itself, in the SNV
and the insertion paths. Table-1 output cannot change.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…7f15d

Review follow-up on #274: classify_coding's allele-pattern stop check and
add_start_stop_heuristic_terms, used when a transcript has no usable CDS,
called the table-1 is_stop_codon, so on MT AGA was not a stop and TGA
was. Route them through the transcript's codon table. Moves the
bio-formats pins to #262's review follow-up (397f15d).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Review follow-up on #274: vep116_stop_retained_without_alt_peptide
(VEP's _ins_del_stop_altered_cil) checked the codon left in the original
stop slot with the table-1 is_stop_codon. Use the transcript's table, as
VEP's _codon_table does: on MT a CDS ending in AGA whose final base is
deleted, with an A shifting in from the 3' UTR, keeps AGA in the slot and
is stop_retained.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@mwiewior
mwiewior force-pushed the fix/mt-codon-table-200 branch from 5c10a90 to 3f684d0 Compare October 18, 2026 15:46
@mwiewior
mwiewior changed the base branch from fix/csq-transcript-order-199 to master October 18, 2026 15:46
@mwiewior

Copy link
Copy Markdown
Contributor Author

@codex review

@mwiewior

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Claude finished @mwiewior's task in 18s —— View job


Review of PR #274: MT codon table 2

I found nothing blocking. I read the diff against origin/master and spot-checked the translate_protein_from_cds call sites. I did not build or run the tests.

What looks right

  • Table-2 overrides. translate_codon_in, translate_codon_bytes_in and is_stop_codon_in change only TGA→W, ATA→M and AGA/AGG→*. Everything else falls through to the unchanged table-1 functions, so nuclear transcripts take the same code paths as before.
  • Table 1 where VEP uses it. The remaining plain translate_protein_from_cds callers are the HGVSp alternate-protein paths (lines 5119, 5220, 5384, 5614, 6006, 6108). This matches the documented _get_fs_peptides, _get_del_peptides and _stop_loss_extra_AA behaviour, and the MT:10121 and MT:14488 evidence.
  • Earlier Codex P2 comments. The no-CDS fallback, classify_coding, add_start_stop_heuristic_terms and the CIL stop-slot fallback now use is_stop_codon_in(tx.codon_table, …). Each has a regression test.
  • start_retained. The literal-ATG check fixes MT:8529 and is identical to the old check under table 1.
  • Null codon_table on MT. It resolves to table 2, so published pre-perf(vep): scale the LazyFrame run pool with workers (output budget, stateful run floor) #262 caches work unchanged. Replacing is_mitochondrial_chrom with the shared is_mitochondrial_contig is a clean dedupe.

Minor, non-blocking

  1. Mixed tables at line 5384. old_aas comes from reference_aas_for_consequence (table-aware), while new_aas uses table 1. If anything later compares the two outside the edited window, an ATA, TGA or AGR codon would differ between the sides. If this mirrors VEP (reference from the cached peptide, alternate from BioPerl's default translate()), a one-line comment would help the next reader.
  2. Case sensitivity. translate_codon_in and translate_codon_bytes_in match uppercase only, like their table-1 counterparts. is_stop_codon_in uppercases its input, so the three are not consistent. This only matters if a cache ever held lowercase CDS.
  3. Unknown table ids. from_ncbi_id silently maps any id other than 2 to Standard. That is fine for human caches. A one-time warning would make other tables easier to spot later.
  4. Extra test. Assert is_stop_codon_in(t, c) == (translate_codon_in(t, c) == Some('*')) over all 64 codons under both tables. That would pin the hard-coded TAA|TAG|AGA|AGG|* list, which the existing 64-codon test does not cover.

Tests
Coverage is good: ATA start, AGA stop, TGA as Trp, MT:8529, the 64-codon diff, the null-on-MT fallback, the stop heuristic and the CIL stop slot. The remaining 26 of 3,134 ClinVar chrMT records are attributed to separate issues, which fits a scoped fix.

No changes requested.
· branch fix/mt-codon-table-200

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 3f684d0d3a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

datafusion-bio-formats#262 was squash-merged as abac8df, whose tree is
identical to the PR head 397f15d these pins named. Moving all seven
formats lines (and the lockfile) to the merge commit keeps no pin on a
commit that exists only on a branch. No code change.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@mwiewior
mwiewior merged commit 9936c6e into master Oct 19, 2026
2 checks passed
mwiewior added a commit that referenced this pull request Oct 19, 2026
…rcent-encoding

master pins formats abac8df for #274; this branch keeps the formats#266
head 64c4001, which already contains abac8df (#262) together with the
#207 writer fix and the #263 Character INFO reader fix. All seven
formats lines and Cargo.lock stay on 64c4001.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
mwiewior added a commit that referenced this pull request Nov 14, 2026
Move every datafusion-bio-format-* dependency (vep and pileup) from the
pre-release rev to tag v1.16.0, which contains the DataFusion 55.2 bump
(biodatageeks/datafusion-bio-formats#248) and the OpenDAL 0.59.5 fix for
remote reads that retried forever under tokio 1.54 (#274). The lockfile
keeps a single copy of datafusion-bio-format-core, datafusion, arrow and
parquet, and drops the OpenDAL git revision.

Also import Session and PhysicalPlanningContext in the ranges planners
instead of spelling out their paths inline (review nit).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
mwiewior added a commit that referenced this pull request Nov 14, 2026
…332)

* chore: bump DataFusion to 55.2.0 and bio-formats to its DF 55 branch

Move the workspace from DataFusion 53.0.0 to 55.2.0 (arrow/parquet 59.3.2,
rust-toolchain and CI on 1.94.0, DataFusion 55.2's MSRV) and pin every
datafusion-bio-format-* dependency to biodatageeks/datafusion-bio-formats#248
(d1eb0c3), which carries the same bump rebased on formats v1.15.0, so it
includes everything the previous e0379fb pin did. pileup and vep share that
one rev so the graph holds one copy of datafusion-bio-format-core and
DataFusion.

Mechanical fallout of the 54/55 API changes:
- `as_any` removed from DataFusion traits: dropped from every impl, the
  `.as_any()` hop removed at plan/expr downcast sites (including the
  multi-line chains in the interval planner), unused `Any` imports
  removed. fastqc's own `QcModule::as_any` is unrelated and kept.
- `ExecutionPlan::apply_expressions` required: leaf implementations for
  the nodes without expressions; `IntervalJoinExec` visits its join keys
  and filter like DataFusion's own `HashJoinExec`.
- `QueryPlanner` / `PhysicalPlanner` take `&dyn Session`, and
  `create_physical_expr` gains the `PhysicalPlanningContext` argument.
- `create_hashes` takes DataFusion's `HashState`; the interval join uses
  `datafusion::common::hash_utils::RandomState` (ahash stays for the
  plain hash maps).
- `Distribution::HashPartitioned` -> `KeyPartitioned`.
- `MemoryPool` now requires `Display` and `name()`: TracingPool and the
  test-only RejectHashPool.
- `MutableArrayData::extend` is deprecated in arrow 59.3: the SIFT page
  gather uses `try_extend` and propagates the overflow error.
- `partition_statistics` returns `Arc<Statistics>` and is deprecated for
  callers; the interval join estimator and the plugin-cache join
  selector read statistics through `StatisticsContext`.

That last item is the one behavioural change. Under DataFusion 55 the
deprecated `partition_statistics` returns unknown statistics for
DataFusion's own nodes, so `choose_for_hash_plan` would have reported
MissingStatistics and silently forced every plugin-cache tier join onto
SortMergeJoin; the adaptive-join tests cover it.

Why: DataFusion 53's external sorter frees its merge headroom and then
re-acquires it from an empty reservation, which starves under
`FairSpillPool` when several partitions sort concurrently (#235).
apache/datafusion#20642 fixes that and shipped in 54.0.0.

Not touched: `research/lance_encoding_sandbox` is outside the workspace
and still pins arrow 58 / datafusion-common 53.

Supersedes #236 and dependabot #8.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* refactor: address review nits on the DataFusion 55.2 bump

- interval join: say why `hash_utils::RandomState::default()` is safe
  (foldhash `FixedState` with a compile-time constant seed, shared by
  both join sides), and fold `partition_statistics_inner` back into
  `partition_statistics` where it lived before the bump, with a note on
  why the partition argument is ignored. Drop the stray blank line left
  by the `Any` import removal.
- plugin-cache join selector: import `StatisticsContext` /
  `StatisticsArgs` instead of spelling the full paths inline.
- the ten leaf `apply_expressions` impls (fastqc, pileup, ranges, vep):
  use imported `Arc` / `PhysicalExpr` / `Result` / `TreeNodeRecursion`
  instead of fully qualified paths.

No behaviour change.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* chore: re-pin datafusion-bio-formats to 99d7f81

Follow biodatageeks/datafusion-bio-formats#248 to its review-nit commit
(test renames, shorter `apply_expressions` signatures, CHANGELOG arrow
version). No API change; vep and pileup stay on one formats rev.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* chore: pin datafusion-bio-formats to the v1.16.0 release

Move every datafusion-bio-format-* dependency (vep and pileup) from the
pre-release rev to tag v1.16.0, which contains the DataFusion 55.2 bump
(biodatageeks/datafusion-bio-formats#248) and the OpenDAL 0.59.5 fix for
remote reads that retried forever under tokio 1.54 (#274). The lockfile
keeps a single copy of datafusion-bio-format-core, datafusion, arrow and
parquet, and drops the OpenDAL git revision.

Also import Session and PhysicalPlanningContext in the ranges planners
instead of spelling out their paths inline (review nit).

Co-Authored-By: Claude Opus 5.5 <[email protected]>

---------

Co-authored-by: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant