fix: update consequences to match mehari 0.43.2 (#2630)#2633
Open
stolpeo wants to merge 5 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (27)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughAdds eight new variant consequence types and propagates them from the protobuf contract through regenerated Python protobufs, backend enums/mappings/presets, JSON schemas and forms, frontend types/labels/UI fields, pinned build deps, and updated tests/snapshots. ChangesVariant Consequence Expansion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
deps-report 🔍Commit scanned: 9afc617 Vulnerable dependencies8 dependencies have vulnerabilities 😱
Outdated dependencies68 outdated dependencies found (including 22 outdated major versions)😢
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
backend/Pipfile (1)
98-99: ⚡ Quick winAdd a floor constraint to protobuf runtime matching the generated code requirement.
The generated code explicitly requires protobuf
5.28.1(viaValidateProtobufRuntimeVersioncalls), but the source Pipfile leaves protobuf unconstrained (*at line 35). While the lock file currently resolves to5.29.6, constraining the source (e.g.,protobuf = ">=5.28.1") prevents import-time errors across different dependency resolution scenarios.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/Pipfile` around lines 98 - 99, The Pipfile currently leaves the protobuf runtime unconstrained while the generated code calls ValidateProtobufRuntimeVersion and requires protobuf >= 5.28.1; update the Pipfile's protobuf entry to add a floor constraint (e.g., set protobuf = ">=5.28.1") so the runtime matches what the generated code (ValidateProtobufRuntimeVersion) expects and avoids import-time version errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/variants/tests/data/query_settings.py`:
- Around line 1-3: The file currently contains a Git LFS pointer (the lines
starting with "version https://git-lfs.github.com/spec/v1" and the oid/size
entries) which is not valid Python and causes a SyntaxError; replace those LFS
pointer lines with the original Python fixture content that was intended to live
in backend/variants/tests/data/query_settings.py, or if the fixture is a
binary/large asset, move it out of a .py file (e.g., to
backend/variants/tests/data/query_settings.json or .txt) and update any
loader/tests that import or open query_settings.py to point to the new file;
ensure the resulting file contains valid Python (or update imports to parse the
new format) and remove the LFS pointer lines entirely.
In `@frontend/src/variants/components/FilterForm/EffectPane.fields.js`:
- Around line 283-288: Update the three non-coding consequence field objects to
include the correct Sequence Ontology IDs and explanations: for the object with
id '3_prime_UTR_variant' set so to 'SO:0001624' and explanation to "A UTR
variant of the 3' UTR"; for '5_prime_UTR_variant' set so to 'SO:0001623' and
explanation to "A UTR variant of the 5' UTR"; and for
'non_coding_transcript_variant' set so to 'SO:0001619' (retain or add an
appropriate explanation if missing). Ensure you update the objects identified by
those id values in EffectPane.fields.js.
- Around line 76-82: Update the Sequence Ontology (SO) metadata for the coding
consequence entries: set the so value for the entry with id
'incomplete_terminal_codon_variant' to "SO:0001626" instead of the placeholder
"SO:", and for the entries with ids 'selenocysteine_gain' and
'selenocysteine_loss' either replace them with appropriate SO terms (e.g., use
"SO:0000885" for stop_codon_redefined_as_selenocysteine or "SO:0002054" for
loss_of_function_variant) or explicitly mark those two ids as custom/non-SO
labels in the UI metadata so it’s clear they are not official SO terms.
---
Nitpick comments:
In `@backend/Pipfile`:
- Around line 98-99: The Pipfile currently leaves the protobuf runtime
unconstrained while the generated code calls ValidateProtobufRuntimeVersion and
requires protobuf >= 5.28.1; update the Pipfile's protobuf entry to add a floor
constraint (e.g., set protobuf = ">=5.28.1") so the runtime matches what the
generated code (ValidateProtobufRuntimeVersion) expects and avoids import-time
version errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a34ba5fd-00e8-44af-bb08-a192c8d97540
⛔ Files ignored due to path filters (1)
backend/Pipfile.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
backend/Makefilebackend/Pipfilebackend/protos/seqvars/protos/query.protobackend/seqvars/factory_defaults.pybackend/seqvars/models/base.pybackend/seqvars/models/protobufs.pybackend/seqvars/protos/output_pb2.pybackend/seqvars/protos/output_pb2.pyibackend/seqvars/protos/query_pb2.pybackend/seqvars/protos/query_pb2.pyibackend/seqvars/tests/snapshots/snap_test_factory_defaults.pybackend/varfish/tests/drf_openapi_schema/varfish_api_schema.yamlbackend/variants/forms.pybackend/variants/query_presets.pybackend/variants/query_schemas.pybackend/variants/schemas/case-query-v1.jsonbackend/variants/tests/data/query_settings.pybackend/variants/tests/factories.pybackend/variants/tests/test_queries.pybackend/variants/tests/test_query_presets.pybackend/variants/tests/test_ui.pybackend/variants/tests/test_views_api.pyfrontend/ext/varfish-api/src/lib/types.gen.tsfrontend/src/seqvars/components/PresetsEditor/lib/consequence.tsfrontend/src/seqvars/components/QueryResults/QueryResultsTable.vuefrontend/src/variants/components/FilterForm/EffectPane.fields.js
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2633 +/- ##
=====================================
Coverage 89% 89%
=====================================
Files 692 692
Lines 40727 40753 +26
=====================================
+ Hits 36576 36602 +26
Misses 4151 4151
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/tests/variants/unit/components/FilterForm/EffectPane.spec.js (1)
129-361:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd test coverage for untested effect fields.
The "effects detailed" test covers 43 of the 51 defined detailed effect fields. Eight fields are missing test coverage:
feature_elongationgene_variantincomplete_terminal_codon_variantnon_coding_transcript_variantprotein_altering_variantrare_amino_acid_variantselenocysteine_gainselenocysteine_lossAdd
wrapper.get(),setValue(), and assertion statements for these fields to ensure comprehensive coverage and prevent regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/tests/variants/unit/components/FilterForm/EffectPane.spec.js` around lines 129 - 361, Add wrapper.get(), setValue(), and expect assertions for the eight missing detailed effect fields inside the "effects detailed" test: call wrapper.get('#detailed-effect-feature_elongation'), wrapper.get('#detailed-effect-gene_variant'), wrapper.get('#detailed-effect-incomplete_terminal_codon_variant'), wrapper.get('#detailed-effect-non_coding_transcript_variant'), wrapper.get('#detailed-effect-protein_altering_variant'), wrapper.get('#detailed-effect-rare_amino_acid_variant'), wrapper.get('#detailed-effect-selenocysteine_gain'), and wrapper.get('#detailed-effect-selenocysteine_loss'), assign them to variables following the existing naming convention (e.g., detailedEffectFeatureElongation), await each .setValue(), and add expect(<variable>.element.checked).toBeTruthy() assertions to match the other detailedEffect checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/tests/variants/unit/components/FilterForm/EffectPane.spec.js`:
- Line 18: The test in EffectPane.spec.js asserts
expect(wrapper.findAll('input').length).toBe(67) but EffectPane.fields.js
defines 66 field IDs; update the test assertion to .toBe(66) unless you
intentionally added a new field—if so, add the missing field definition to
EffectPane.fields.js to bring the runtime input count to 67; locate the
assertion in EffectPane.spec.js and verify against the list in
EffectPane.fields.js to keep numbers consistent.
---
Outside diff comments:
In `@frontend/tests/variants/unit/components/FilterForm/EffectPane.spec.js`:
- Around line 129-361: Add wrapper.get(), setValue(), and expect assertions for
the eight missing detailed effect fields inside the "effects detailed" test:
call wrapper.get('#detailed-effect-feature_elongation'),
wrapper.get('#detailed-effect-gene_variant'),
wrapper.get('#detailed-effect-incomplete_terminal_codon_variant'),
wrapper.get('#detailed-effect-non_coding_transcript_variant'),
wrapper.get('#detailed-effect-protein_altering_variant'),
wrapper.get('#detailed-effect-rare_amino_acid_variant'),
wrapper.get('#detailed-effect-selenocysteine_gain'), and
wrapper.get('#detailed-effect-selenocysteine_loss'), assign them to variables
following the existing naming convention (e.g.,
detailedEffectFeatureElongation), await each .setValue(), and add
expect(<variable>.element.checked).toBeTruthy() assertions to match the other
detailedEffect checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0df23e91-49d5-442a-a3f8-8272cf53bd33
📒 Files selected for processing (2)
frontend/src/variants/components/FilterForm/EffectPane.fields.jsfrontend/tests/variants/unit/components/FilterForm/EffectPane.spec.js
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/variants/components/FilterForm/EffectPane.fields.js
stolpeo
force-pushed
the
2630-update-consequenecs-to-match-mehari-0432
branch
from
826177e to
9afc617
Compare
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
1 participant
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.
Summary by CodeRabbit
New Features
Tests
Chores