-
Notifications
You must be signed in to change notification settings - Fork 0
Enhancement/ruff updates #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
iimpulse
commented
Jan 14, 2026
- Update stubs so server runs and passes verifier.
- remove black closes Update failing tests and review code #1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the Beacon v2 API implementation to achieve beacon-verifier compliance and modernizes the development tooling by replacing Black with Ruff for code formatting.
Changes:
- Updated stubs to enable the server to start and pass beacon-verifier compliance tests
- Removed Black formatter in favor of Ruff's built-in formatting capabilities
- Updated API endpoints to use Beacon v2-compliant camelCase field naming
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_main.py | Updated test endpoints to match new API paths and response formats |
| tests/test_info.py | Updated assertions to use camelCase field names per Beacon v2 spec |
| tests/test_filter_parsing.py | Added comprehensive filter parsing tests for Beacon v2 formats |
| src/beacon_api/services/stubs.py | New stub service implementations for verifier compliance |
| src/beacon_api/services/base.py | Refactored service interfaces to use modern type hints |
| src/beacon_api/models/response.py | Added camelCase aliases for Beacon v2 compliance |
| src/beacon_api/models/request.py | Updated FilteringTerm model with improved documentation |
| src/beacon_api/api/query_params.py | New query parameter parsing utilities |
| src/beacon_api/api/info.py | Updated response serialization to use camelCase |
| src/beacon_api/core/config.py | Updated configuration with modern pydantic-settings syntax |
| pyproject.toml | Removed Black configuration and added Ruff formatter settings |
| README.md | Updated documentation to reference Ruff instead of Black |
| Dockerfile | Optimized build process and updated health check endpoint |
| .github/workflows/ci.yml | Updated CI to use Ruff formatter and improved Docker testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| assert len(result) == 1 | ||
| assert result[0].id == "HP:0001250" | ||
| assert result[0].includeDescendantTerms is False |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field name includeDescendantTerms uses camelCase, but the FilteringTerm model field is defined as includeDescendantTerms (camelCase) in line 27 of src/beacon_api/models/request.py, which is correct for the Beacon v2 spec. However, when accessing the parsed result in Python, the field should be accessed using the Python attribute name. Verify that the FilteringTerm model correctly maps this field for both serialization and deserialization.
| assert result[0].includeDescendantTerms is False | |
| assert result[0].include_descendant_terms is False |
| default=False, | ||
| description="Include descendant terms in the query", | ||
| includeDescendantTerms: bool = Field( | ||
| default=True, |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value for includeDescendantTerms is set to True, but in line 63 of tests/test_filter_parsing.py, the test expects False when this field is explicitly set to false in the input. Ensure the default value aligns with the Beacon v2 specification. According to the spec, the default should typically be true, so this appears correct, but verify that test expectations match the intended behavior.
| environment: str = Field( | ||
| default="development", | ||
| description="Environment: production, development, or staging", | ||
| default="dev", |
Copilot
AI
Jan 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default environment changed from 'development' to 'dev', and line 47 in src/beacon_api/models/response.py shows examples including 'dev'. Ensure all references to environment values in documentation and examples are consistent with this change.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>