Conversation
Reviewer's GuideThis PR adds logging of current and previous coverage percentages across both Python and Rust scripts in the generate-coverage action, exposes new baseline file inputs in the action metadata, and updates documentation to reflect the new behavior. Class diagram for updated Python coverage scriptclassDiagram
class run_python {
+main(output_path, lang, fmt, github_output, baseline_file)
+read_previous(baseline)
}
run_python : +main()
run_python : +read_previous()
Class diagram for updated Rust coverage scriptclassDiagram
class run_rust {
+main(output_path, features, with_default_features, lang, fmt, github_output, baseline_file)
+read_previous(baseline)
}
run_rust : +main()
run_rust : +read_previous()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 57 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughUpdate the GitHub Action Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Action
participant Rust Script
participant Python Script
participant Baseline File
GitHub Action->>Rust Script: Run with BASELINE_RUST_FILE env
Rust Script->>Baseline File: Read previous coverage (if exists)
Rust Script->>GitHub Action: Log current and previous coverage
GitHub Action->>Python Script: Run with BASELINE_PYTHON_FILE env
Python Script->>Baseline File: Read previous coverage (if exists)
Python Script->>GitHub Action: Log current and previous coverage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.
- The new baseline-python-file and baseline-rust-file variables aren’t declared under inputs in action.yml—add those input definitions so the environment vars are actually set.
- The read_previous function is duplicated in both scripts; consider extracting that logic into a shared utility to reduce maintenance overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new baseline-python-file and baseline-rust-file variables aren’t declared under inputs in action.yml—add those input definitions so the environment vars are actually set.
- The read_previous function is duplicated in both scripts; consider extracting that logic into a shared utility to reduce maintenance overhead.
## Individual Comments
### Comment 1
<location> `.github/actions/generate-coverage/scripts/run_python.py:63` </location>
<code_context>
xml_tmp.unlink(missing_ok=True)
+def read_previous(baseline: Path | None) -> str | None:
+ """Return the previously stored coverage percentage if available."""
+ if baseline and baseline.is_file():
+ try:
+ return f"{float(baseline.read_text().strip()):.2f}"
</code_context>
<issue_to_address>
Consider handling OSError when reading the baseline file.
Only ValueError is currently handled. Catching OSError as well would improve robustness against file access issues.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
def read_previous(baseline: Path | None) -> str | None:
"""Return the previously stored coverage percentage if available."""
if baseline and baseline.is_file():
try:
return f"{float(baseline.read_text().strip()):.2f}"
except ValueError:
return None
return None
=======
def read_previous(baseline: Path | None) -> str | None:
"""Return the previously stored coverage percentage if available."""
if baseline and baseline.is_file():
try:
return f"{float(baseline.read_text().strip()):.2f}"
except (ValueError, OSError):
return None
return None
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `.github/actions/generate-coverage/scripts/run_rust.py:102` </location>
<code_context>
xml_tmp.unlink(missing_ok=True)
+def read_previous(baseline: Path | None) -> str | None:
+ """Return the previously stored coverage percentage if available."""
+ if baseline and baseline.is_file():
+ try:
+ return f"{float(baseline.read_text().strip()):.2f}"
</code_context>
<issue_to_address>
Consider catching OSError when reading the baseline file.
Currently, only ValueError is handled. To prevent unhandled exceptions from I/O errors, also catch OSError when reading the file.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
try:
return f"{float(baseline.read_text().strip()):.2f}"
except ValueError:
return None
=======
try:
return f"{float(baseline.read_text().strip()):.2f}"
except (ValueError, OSError):
return None
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/actions/generate-coverage/CHANGELOG.md(1 hunks).github/actions/generate-coverage/README.md(1 hunks).github/actions/generate-coverage/action.yml(2 hunks).github/actions/generate-coverage/scripts/run_python.py(3 hunks).github/actions/generate-coverage/scripts/run_rust.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
.github/actions/*/CHANGELOG.md
📄 CodeRabbit Inference Engine (AGENTS.md)
Each action must have a
CHANGELOG.mdthat follows SemVer-based changelog for that action only.
Files:
.github/actions/generate-coverage/CHANGELOG.md
.github/actions/*/README.md
📄 CodeRabbit Inference Engine (AGENTS.md)
.github/actions/*/README.md: Each action must have aREADME.mdcontaining a one-liner summary, table of inputs, table of outputs, usage example withuses: ./.github/actions/<name>@<major>, and a release history link to CHANGELOG.
Add aDEPRECATED:banner to README and repository description when deprecating an action.
Files:
.github/actions/generate-coverage/README.md
.github/actions/*/action.yml
📄 CodeRabbit Inference Engine (AGENTS.md)
.github/actions/*/action.yml: Each action must have anaction.ymlfile containing every input and output, with required ones clearly marked.
Composite actions referencing sibling scripts must use${{ github.action_path }}for portability.
Files:
.github/actions/generate-coverage/action.yml
🧬 Code Graph Analysis (1)
.github/actions/generate-coverage/scripts/run_rust.py (1)
.github/actions/generate-coverage/scripts/run_python.py (1)
read_previous(63-70)
🪛 LanguageTool
.github/actions/generate-coverage/README.md
[typographical] ~102-~102: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...the previous percentage is shown as well. Release history is available in [CHANG...
(WRB_QUESTION_MARK)
🔇 Additional comments (11)
.github/actions/generate-coverage/CHANGELOG.md (1)
3-7: LGTM! Changelog entry follows required format.The changelog entry correctly documents the new logging functionality and follows the SemVer-based format as required by the coding guidelines.
.github/actions/generate-coverage/README.md (1)
100-103: LGTM! Documentation accurately describes the new logging behaviour.The addition clearly explains when coverage percentages are logged, enhancing user understanding of the action's output.
.github/actions/generate-coverage/action.yml (2)
96-96: LGTM! Environment variable correctly passes baseline file path.The
BASELINE_RUST_FILEenvironment variable properly passes the input parameter to the Rust coverage script.
146-146: LGTM! Environment variable correctly passes baseline file path.The
BASELINE_PYTHON_FILEenvironment variable properly passes the input parameter to the Python coverage script..github/actions/generate-coverage/scripts/run_python.py (4)
28-28: LGTM! Baseline option correctly configured.The
BASELINE_OPTproperly reads from theBASELINE_PYTHON_FILEenvironment variable with appropriate default handling.
63-71: LGTM! Baseline reading function is well-implemented.The
read_previousfunction correctly handles file existence, reading, and parsing with proper error handling for invalid values.
78-78: LGTM! Function signature correctly updated.The
baseline_fileparameter is properly added with the correct type annotation and default value.
99-103: LGTM! Coverage logging implementation is clear and informative.The logging output provides valuable information about current coverage and conditionally displays previous coverage when available.
.github/actions/generate-coverage/scripts/run_rust.py (3)
34-34: LGTM! Baseline option correctly configured.The
BASELINE_OPTproperly reads from theBASELINE_RUST_FILEenvironment variable with appropriate default handling.
120-120: LGTM! Function signature correctly updated.The
baseline_fileparameter is properly added with the correct type annotation and default value.
145-149: LGTM! Coverage logging implementation is clear and informative.The logging output provides valuable information about current coverage and conditionally displays previous coverage when available, maintaining consistency with the Python implementation.
leynos
force-pushed
the
codex/log-current-and-previous-coverage-percentage
branch
from
f836e97 to
8fd4663
Compare
1 participant
Summary
Testing
make testmake linthttps://chatgpt.com/codex/tasks/task_e_6887e4e1207c83229ddaa667448c5a35
Summary by Sourcery
Log coverage metrics during the generate-coverage action by printing the current percentage and, when a baseline file is available, the previous percentage. Extend the action inputs and scripts to support a baseline file for ratcheting and update documentation accordingly.
New Features:
Enhancements:
baseline-python-fileandbaseline-rust-fileinputs to the GitHub Action and pass them as environment variables to the coverage scriptsDocumentation: