Add ratcheting option to generate-coverage action#24
Conversation
Reviewer's GuideThis PR introduces optional ratcheting support for the generate-coverage action—tracking and enforcing separate Rust and Python coverage baselines—enhances both runners to extract and expose coverage percentages, and updates documentation and version accordingly. Class diagram for updated run_rust.py and run_python.py scriptsclassDiagram
class run_rust {
+main(output_path, features, fmt, with_default)
+get_cargo_coverage_cmd(fmt, out, features, with_default)
+extract_percent(output)
}
class run_python {
+main(output_path, lang, fmt)
+coverage_cmd_for_fmt(fmt, out)
+percent_from_xml(xml_file)
}
Flow diagram for ratcheting logic in generate-coverage actionflowchart TD
Start([Start])
Detect[Detect language]
RatchetCheck{with-ratchet enabled?}
LangCheck{Language: Rust, Python, or Mixed?}
RestoreBaseline[Restore baseline from cache]
RunCoverage[Run coverage script]
ExtractPercent[Extract coverage percent]
Ratchet[Run ratchet_coverage.py]
SaveBaseline[Save new baseline to cache]
End([End])
Start --> Detect --> RatchetCheck
RatchetCheck -- No --> RunCoverage
RatchetCheck -- Yes --> LangCheck
LangCheck -- Rust or Mixed --> RestoreBaseline
LangCheck -- Python or Mixed --> RestoreBaseline
RestoreBaseline --> RunCoverage
RunCoverage --> ExtractPercent --> Ratchet
Ratchet --> SaveBaseline --> End
RunCoverage --> End
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update introduces a coverage ratcheting feature to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Action
participant Rust Script
participant Python Script
participant Baseline Cache
User->>GitHub Action: Trigger workflow (with-ratchet enabled)
GitHub Action->>Baseline Cache: Restore baseline files (Rust & Python)
GitHub Action->>Rust Script: Run coverage (if Rust present)
Rust Script->>GitHub Action: Output coverage percent
GitHub Action->>Python Script: Run coverage (if Python present)
Python Script->>GitHub Action: Output coverage percent
GitHub Action->>GitHub Action: Compare coverage to baseline(s)
alt Coverage decreased
GitHub Action->>User: Fail workflow
else Coverage maintained/increased
GitHub Action->>Baseline Cache: Update and save new baseline(s)
GitHub Action->>User: Workflow succeeds
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (2)`.github/actions/*/{action.yml,README.md,CHANGELOG.md,src/,tests/}`: Each action...
📄 Source: CodeRabbit Inference Engine (AGENTS.md) List of files the instruction was applied to:
`.github/actions/*/CHANGELOG.md`: Each action must have a CHANGELOG.md that follows SemVer-based changelog for this action only
📄 Source: CodeRabbit Inference Engine (AGENTS.md) List of files the instruction was applied to:
🔇 Additional comments (13)
✨ 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 - here's some feedback:
- Make sure the
actions/cacherestore/save steps handle first-run (missing baseline file) gracefully—e.g. initialize the baseline file so the cache step actually picks it up. - There’s a lot of duplicated if-condition logic for Rust vs Python ratcheting in action.yml—consider consolidating or templating those steps to reduce verbosity and chance for drift.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Make sure the `actions/cache` restore/save steps handle first-run (missing baseline file) gracefully—e.g. initialize the baseline file so the cache step actually picks it up.
- There’s a lot of duplicated if-condition logic for Rust vs Python ratcheting in action.yml—consider consolidating or templating those steps to reduce verbosity and chance for drift.
## Individual Comments
### Comment 1
<location> `.github/actions/generate-coverage/action.yml:77` </location>
<code_context>
+ INPUT_BASELINE_FILE: ${{ inputs.baseline-rust-file }}
+ CURRENT_PERCENT: ${{ steps.rust.outputs.percent }}
+ shell: bash
+ - name: Save Rust baseline
+ if: success() && inputs.with-ratchet == 'true' && (steps.detect.outputs.lang == 'rust' || steps.detect.outputs.lang == 'mixed')
+ uses: actions/cache@v4
+ with:
+ path: ${{ inputs.baseline-rust-file }}
+ key: ratchet-baseline-rust-${{ runner.os }}
- name: Install uv and set the python version
if: steps.detect.outputs.lang == 'python' || steps.detect.outputs.lang == 'mixed'
</code_context>
<issue_to_address>
Saving the baseline with the same cache key as restore may prevent updates.
GitHub Actions cache does not overwrite existing keys, so using the same key for saving and restoring may prevent updates. Use a different key for saving or implement a restore-keys strategy to ensure the baseline updates correctly.
</issue_to_address>
### Comment 2
<location> `.github/actions/generate-coverage/action.yml:115` </location>
<code_context>
+ INPUT_BASELINE_FILE: ${{ inputs.baseline-python-file }}
+ CURRENT_PERCENT: ${{ steps.python.outputs.percent }}
+ shell: bash
+ - name: Save Python baseline
+ if: success() && inputs.with-ratchet == 'true' && (steps.detect.outputs.lang == 'python' || steps.detect.outputs.lang == 'mixed')
+ uses: actions/cache@v4
+ with:
+ path: ${{ inputs.baseline-python-file }}
+ key: ratchet-baseline-python-${{ runner.os }}
- if: steps.detect.outputs.lang == 'mixed'
run: uv run --script scripts/generate_coverage/merge_cobertura.py
</code_context>
<issue_to_address>
Python baseline cache save uses the same key as restore, which may block updates.
Since GitHub Actions cache does not overwrite existing keys, this setup may prevent the Python baseline from being updated. Please use a different cache key strategy to ensure the baseline can be refreshed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| uses: actions/cache@v4 | ||
| with: | ||
| path: ${{ inputs.baseline-rust-file }} | ||
| key: ratchet-baseline-rust-${{ runner.os }} |
There was a problem hiding this comment.
issue (bug_risk): Saving the baseline with the same cache key as restore may prevent updates.
GitHub Actions cache does not overwrite existing keys, so using the same key for saving and restoring may prevent updates. Use a different key for saving or implement a restore-keys strategy to ensure the baseline updates correctly.
| uses: actions/cache@v4 | ||
| with: | ||
| path: ${{ inputs.baseline-python-file }} | ||
| key: ratchet-baseline-python-${{ runner.os }} |
There was a problem hiding this comment.
issue (bug_risk): Python baseline cache save uses the same key as restore, which may block updates.
Since GitHub Actions cache does not overwrite existing keys, this setup may prevent the Python baseline from being updated. Please use a different cache key strategy to ensure the baseline can be refreshed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.github/actions/generate-coverage/action.yml (1)
92-100: Address cache key strategy to enable baseline updates.The current cache key strategy may prevent baseline updates as highlighted in past reviews. Using the same key for restore and save operations can prevent cache updates.
Consider using a more sophisticated cache key strategy:
- key: ratchet-baseline-${{ runner.os }}-${{ github.run_id }} - restore-keys: ratchet-baseline-${{ runner.os }}- + key: ratchet-baseline-${{ runner.os }}-${{ github.run_id }} + restore-keys: | + ratchet-baseline-${{ runner.os }}-This approach uses a unique key with the run ID for saving whilst allowing restoration from any previous baseline.
📜 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(2 hunks).github/actions/generate-coverage/action.yml(2 hunks)scripts/generate_coverage/run_python.py(3 hunks)scripts/generate_coverage/run_rust.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
`.github/actions/*/{action.yml,README.md,CHANGELOG.md,src/,tests/}`: Each action...
.github/actions/*/{action.yml,README.md,CHANGELOG.md,src/,tests/}: Each action must have its own directory under .github/actions/, containing action.yml, README.md, src/, tests/, and CHANGELOG.md
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
.github/actions/generate-coverage/CHANGELOG.md.github/actions/generate-coverage/README.md.github/actions/generate-coverage/action.yml
`.github/actions/*/CHANGELOG.md`: Each action must have a CHANGELOG.md that follows SemVer-based changelog for this action only
.github/actions/*/CHANGELOG.md: Each action must have a CHANGELOG.md that follows SemVer-based changelog for this action only
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
.github/actions/generate-coverage/CHANGELOG.md
`.github/actions/*/README.md`: Each action's README.md must contain a one-liner ...
.github/actions/*/README.md: Each action's README.md must contain a one-liner summary, table of inputs, table of outputs, usage example, and release history link to CHANGELOG
Add a DEPRECATED: banner to README and repository description when deprecating an action
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
.github/actions/generate-coverage/README.md
`.github/actions/*/action.yml`: Fill in action.yml with every input and output; ...
.github/actions/*/action.yml: Fill in action.yml with every input and output; mark required ones clearly
For composite actions and path context, use "${{ github.action_path }}" when referencing sibling scripts for portability
📄 Source: CodeRabbit Inference Engine (AGENTS.md)
List of files the instruction was applied to:
.github/actions/generate-coverage/action.yml
🪛 Ruff (0.11.9)
scripts/generate_coverage/run_python.py
42-42: Using xml to parse untrusted data is known to be vulnerable to XML attacks; use defusedxml equivalents
(S314)
74-74: Trailing comma missing
Add trailing comma
(COM812)
🔇 Additional comments (13)
.github/actions/generate-coverage/CHANGELOG.md (1)
3-9: Well-structured changelog entry.The changelog entry clearly documents the new ratcheting feature with proper versioning and follows the established format.
.github/actions/generate-coverage/README.md (2)
39-41: Comprehensive documentation for new ratcheting inputs.The new input parameters are clearly documented with appropriate descriptions and sensible defaults.
90-98: Clear usage example for ratcheting feature.The example demonstrates proper usage of the new ratcheting functionality in a concise manner.
scripts/generate_coverage/run_rust.py (4)
9-9: Import addition for regex functionality.The
remodule import is correctly added to support coverage percentage extraction.
27-27: Appropriate addition of summary-only flag.The
--summary-onlyflag is correctly added to facilitate coverage percentage extraction from the output.
62-69: Improved error handling and output processing.The changes to use
.run(retcode=None)and explicit return code checking improve error handling. The addition of stdout output and percentage extraction is well-implemented.
73-73: Proper output of coverage percentage.The coverage percentage is correctly written to the GitHub Actions output for use in ratcheting steps.
scripts/generate_coverage/run_python.py (3)
39-48: Well-implemented XML parsing function.The
percent_from_xmlfunction correctly extracts coverage percentages from Cobertura XML files with proper error handling.
69-82: Proper handling of different coverage formats.The logic correctly handles both
coveragepyand other formats, generating temporary XML files when needed and extracting percentages appropriately.
85-85: Correct output of coverage percentage.The coverage percentage is properly written to the GitHub Actions output for ratcheting functionality.
.github/actions/generate-coverage/action.yml (3)
22-34: Well-defined inputs for ratcheting functionality.The new inputs are properly defined with clear descriptions, appropriate types, and sensible defaults.
51-70: Comprehensive baseline management steps.The restore and ensure baseline steps are well-implemented with proper conditional logic and file creation.
78-91: Proper ratcheting logic for both languages.The ratcheting step correctly handles both Rust and Python coverage separately based on the detected language.
leynos
deleted the
codex/integrate-ratcheting-coverage-into-generate-coverage
branch
1 participant
Summary
with-ratchetoption to generate-coverageTesting
python -m py_compile scripts/generate_coverage/run_rust.py scripts/generate_coverage/run_python.pyhttps://chatgpt.com/codex/tasks/task_e_6869cc7ccf608322b3d4dfdce9ebae08
Summary by Sourcery
Add optional ratcheting support to the generate-coverage action by tracking separate Rust and Python baselines, enforcing coverage retention, emitting coverage percentages, and updating documentation.
New Features:
Enhancements:
CI:
Documentation: