Set up CI pipeline with GitHub Actions #9 - #23
Conversation
This CI workflow runs on pushes to the main branch and pull requests. It sets up Python 3.12, installs dependencies, performs linting, formatting checks, type checks, runs tests, and uploads a coverage report.
Added Pyright configuration for type checking.
Added a test to verify FHIRAdapter inherits from EHRAdapter.
📝 WalkthroughWalkthroughAdds a GitHub Actions CI workflow for Python 3.12, configures Pyright, pytest, and coverage, adds a CI badge and ChangesCI and validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
32-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude test files in type checking.
Currently, Pyright only type-checks the
src/directory. It is highly recommended to also type-check your tests (and the newly createdsim/directory) to ensure your test inputs and mocks remain synchronized with the application's type signatures.♻️ Proposed fix
- name: Type check - run: pyright src/ + run: pyright src/ tests/ sim/🤖 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 @.github/workflows/ci.yml around lines 32 - 33, Update the “Type check” workflow step to run Pyright against src/, the test directory, and sim/ so application code, tests, mocks, and simulation code are all type-checked together.tests/test_monitor.py (1)
7-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
isinstance()over stricttype()comparison.In Python, using
isinstance(obj, Class)is more idiomatic than checkingtype(obj) is Class. It cleanly supports inheritance and plays better withpytestassertion rewriting for readable error messages.♻️ Proposed fix
class TestAuditPipeline: def test_audit_pipeline_instantiable(self) -> None: - assert type(AuditPipeline()) is AuditPipeline + assert isinstance(AuditPipeline(), AuditPipeline) class TestBehaviorAnalyzer: def test_behavior_analyzer_instantiable(self) -> None: - assert type(BehaviorAnalyzer()) is BehaviorAnalyzer + assert isinstance(BehaviorAnalyzer(), BehaviorAnalyzer)🤖 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 `@tests/test_monitor.py` around lines 7 - 14, Update the instantiability assertions in test_audit_pipeline and test_behavior_analyzer to use isinstance() with the corresponding class, preserving the existing object construction and assertion intent while allowing subclasses.
🤖 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 @.github/workflows/ci.yml:
- Around line 14-15: Update the actions/checkout step in the CI workflow to set
persist-credentials to false, ensuring the repository token is not retained in
local Git configuration while preserving the existing checkout behavior.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 32-33: Update the “Type check” workflow step to run Pyright
against src/, the test directory, and sim/ so application code, tests, mocks,
and simulation code are all type-checked together.
In `@tests/test_monitor.py`:
- Around line 7-14: Update the instantiability assertions in test_audit_pipeline
and test_behavior_analyzer to use isinstance() with the corresponding class,
preserving the existing object construction and assertion intent while allowing
subclasses.
🪄 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 Plus
Run ID: 33c117e3-db9f-4066-b700-1e416a69105c
📒 Files selected for processing (6)
.github/workflows/ci.ymlREADME.mdpyproject.tomlsim/.gitkeeptests/test_integration.pytests/test_monitor.py
| - name: Checkout | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable credential persistence to prevent token leakage.
By default, actions/checkout persists the repository's GITHUB_TOKEN in the local Git configuration. This introduces a security risk where the token could be exposed if the Git directory is read by untrusted tools or inadvertently packaged into an artifact. Set persist-credentials: false to mitigate this risk.
🔒️ Proposed fix
- name: Checkout
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout | |
| uses: actions/checkout@v4 | |
| - name: Checkout | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 14-15: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/ci.yml around lines 14 - 15, Update the actions/checkout
step in the CI workflow to set persist-credentials to false, ensuring the
repository token is not retained in local Git configuration while preserving the
existing checkout behavior.
Source: Linters/SAST tools
New Features
Added an automated CI workflow that runs linting, type checks, and the test suite with coverage reporting.
Added a CI status badge to the README.
Tests
Expanded smoke tests to verify key integration and monitoring components initialize successfully.
Documentation
Added documentation describing the simulated hospital environment for development and testing.
Summary by CodeRabbit
New Features
Tests
Chores