Conversation
Create a throwaway coverage virtual environment and install `slipcover`, `pytest`, and `coverage` into it with `uv pip --python`.
Avoid relying on the separate `uvx` shim being present on PATH when running formatting and lint targets from automation.
Prefer the common user-local install locations for `uv` and `action-validator` so automation with a narrow PATH can still run the format, lint, and typecheck targets.
OverviewThis PR changes the generate-coverage GitHub Action to create and use a job‑local persistent virtual environment (.venv-coverage) for all Python coverage tooling instead of invoking tools via Key changesCore implementation (.github/actions/generate-coverage/scripts/run_python.py)
Tests (.github/actions/generate-coverage/tests/test_scripts.py)
Makefile and auxiliary scripts
Documentation
Review and remediation items surfaced during review
Tests & validation
SummaryMoves Python coverage tooling to a managed, job‑local WalkthroughCreate and reuse a dedicated Changes
Sequence Diagram(s)sequenceDiagram
participant Script as run_python.py
participant UV as uv
participant Venv as .venv-coverage
participant Tooling as slipcover/pytest/coverage
Script->>Venv: Request cached Python (_coverage_python_cmd)
alt missing or invalid
Script->>UV: Run `uv venv .venv-coverage`
UV-->>Script: venv created
Script->>UV: Run `UV_PROJECT_ENVIRONMENT=... uv sync --inexact --python <venv_python>`
UV-->>Script: deps synced
Script->>UV: Run `uv pip install --python <venv_python> slipcover pytest coverage`
UV-->>Script: tooling installed
else cached valid
Venv-->>Script: Return cached Python
end
Script->>Tooling: Invoke via venv Python (`<venv_python> -m slipcover | -m pytest | -m coverage`)
Tooling-->>Script: Produce coverage artefacts
Possibly related issues
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 2 warnings, 1 inconclusive)
✅ Passed checks (13 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 11 minutes and 52 seconds. Comment |
Reviewer's GuideRun Python coverage generation inside an isolated, lazily-created .venv-coverage virtualenv instead of Sequence diagram for Python coverage venv setup and reusesequenceDiagram
actor GitHubActions
participant generate_coverage_action
participant run_python_script
participant coverage_python_cmd as _coverage_python_cmd
participant create_venv
participant install_tools as install_coverage_tools
participant uv_cli as uv
participant slipcover_pytest_coverage
GitHubActions->>generate_coverage_action: run Python coverage step
generate_coverage_action->>run_python_script: invoke run_python.py
Note over run_python_script,coverage_python_cmd: First coverage invocation
run_python_script->>coverage_python_cmd: request coverage Python
coverage_python_cmd->>create_venv: create_venv()
alt venv_does_not_exist
create_venv->>uv_cli: venv .venv-coverage
uv_cli-->>create_venv: venv created
else venv_exists
create_venv-->>create_venv: reuse existing venv
end
create_venv-->>coverage_python_cmd: venv_python_path
coverage_python_cmd->>install_tools: install_coverage_tools(venv_python_path)
install_tools->>uv_cli: pip install --python venv_python slipcover pytest coverage
uv_cli-->>install_tools: tools installed
install_tools-->>coverage_python_cmd: success
coverage_python_cmd-->>run_python_script: BoundCommand for venv_python
run_python_script->>slipcover_pytest_coverage: run slipcover/pytest/coverage via venv_python
slipcover_pytest_coverage-->>run_python_script: coverage results
run_python_script-->>generate_coverage_action: Cobertura XML
generate_coverage_action-->>GitHubActions: upload coverage artefact
Note over run_python_script,coverage_python_cmd: Subsequent coverage invocation
GitHubActions->>generate_coverage_action: run Python coverage step again
generate_coverage_action->>run_python_script: invoke run_python.py
run_python_script->>coverage_python_cmd: request coverage Python
coverage_python_cmd-->>run_python_script: reuse cached BoundCommand
run_python_script->>slipcover_pytest_coverage: run tools via cached venv_python
slipcover_pytest_coverage-->>run_python_script: coverage results
Flow diagram for coverage venv lifecycle functionsflowchart TD
start[Start coverage tooling setup]
cmd[_coverage_python_cmd]
venv_check[Does .venv-coverage exist]
create[create_venv]
install[install_coverage_tools]
uv_venv[uv venv .venv-coverage]
detect_python[Locate venv python executable]
python_missing[Python executable missing]
recreate[Remove and recreate .venv-coverage]
uv_pip[uv pip install --python venv_python slipcover pytest coverage]
cache_cmd[Cache BoundCommand for venv python]
return_cmd[Return cached BoundCommand]
start --> cmd
cmd -->|_COVERAGE_PYTHON_CMD is None| create
cmd -->|_COVERAGE_PYTHON_CMD is set| return_cmd
create --> venv_check
venv_check -->|no| uv_venv
venv_check -->|yes| detect_python
uv_venv --> detect_python
detect_python -->|found| install
detect_python -->|not found| python_missing
python_missing --> recreate
recreate --> uv_venv
install --> uv_pip
uv_pip --> cache_cmd
cache_cmd --> return_cmd
return_cmd --> end_node[Coverage venv python ready]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Cache the coverage-capable Python command behind one helper, reuse an existing coverage venv when present, and resolve the venv interpreter across POSIX and Windows layouts. Extend the run_python tests to cover cached command reuse and the coverage.py export path using the same venv interpreter.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. .github/actions/generate-coverage/tests/test_scripts.py Comment on file def _get_coverage_cmd_parts(
tmp_path: Path,
run_python_module: ModuleType,
monkeypatch: pytest.MonkeyPatch,❌ New issue: Excess Number of Function Arguments |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Repair `.venv-coverage` when the directory exists without a usable Python interpreter, so coverage setup can recover from incomplete or stale venvs. Reject non-file interpreter placeholders before reusing an existing venv.
This comment was marked as resolved.
This comment was marked as resolved.
Introduce `CoverageFmtSpec` so the coverage command test helper stays within the argument-count limit while preserving the same test behaviour.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Rename unused `fake_tmp_coveragepy_xml` parameters in coveragepy tests so the stubs clearly follow the unused-argument convention.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. Code Duplication.github/actions/generate-coverage/tests/test_scripts.py: What lead to degradation?introduced similar code in: test_create_venv_recreates_broken_coverage_venv,test_create_venv_recreates_invalid_python_candidate,test_create_venv_returns_coverage_python Why does this problem occur?Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health. How to fix it?A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More |
This comment was marked as resolved.
This comment was marked as resolved.
Add observable setup and reuse logging around the coverage venv, expand helper docstrings, and document the venv lifecycle and singleton model. Cover broken-cache recovery and missing interpreter detection in the run_python tests.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Resolve `markdownlint` from the Bun install path when automation runs with a restricted `PATH`, and lint branch-changed Markdown files before falling back to the full tree. Keep the coverage design API table under local Markdown lint disables so the requested table remains readable without surfacing unrelated repository-wide Markdown debt.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/actions/generate-coverage/tests/test_scripts.py (1)
2582-2598: 🧹 Nitpick | 🔵 TrivialCollapse the failure-matrix parameters.
test_run_python_integration_uv_failure_modesstill takes five arguments, so it remains above the CodeScene limit and is harder to extend. Bundlewrite_kwargsandexpected_in_stderrinto a frozen spec dataclass, or split the success and failure cases into separate tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/generate-coverage/tests/test_scripts.py around lines 2582 - 2598, Collapse the multiple parameters by introducing a frozen dataclass (e.g., UVFailureSpec) holding write_kwargs: dict[str,int] and expected_in_stderr: str|None, then change the test signature of test_run_python_integration_uv_failure_modes to accept a single spec: UVFailureSpec instead of separate write_kwargs and expected_in_stderr; update the parametrization that feeds the test to pass instances of UVFailureSpec (or split into two focused tests for success vs failure) and replace all references to write_kwargs and expected_in_stderr inside the test with spec.write_kwargs and spec.expected_in_stderr so the test function only has the single argument.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/actions/generate-coverage/tests/test_scripts.py:
- Around line 2582-2598: Collapse the multiple parameters by introducing a
frozen dataclass (e.g., UVFailureSpec) holding write_kwargs: dict[str,int] and
expected_in_stderr: str|None, then change the test signature of
test_run_python_integration_uv_failure_modes to accept a single spec:
UVFailureSpec instead of separate write_kwargs and expected_in_stderr; update
the parametrization that feeds the test to pass instances of UVFailureSpec (or
split into two focused tests for success vs failure) and replace all references
to write_kwargs and expected_in_stderr inside the test with spec.write_kwargs
and spec.expected_in_stderr so the test function only has the single argument.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c6a001d6-6d5d-4e5b-9985-b8c920370611
📒 Files selected for processing (1)
.github/actions/generate-coverage/tests/test_scripts.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/generate-coverage/scripts/run_python.py (1)
270-274:⚠️ Potential issue | 🟠 MajorCapture setup failures in
mainand preserve subprocess exit codes.Move
coverage_cmd_for_fmt(fmt, out)inside thetryblock.Line 270currently runs venv setup outside theProcessExecutionErrorhandler, souv venv/sync/pipfailures can escape as uncaught exceptions and degrade exit-code propagation.🔧 Proposed fix
- cmd = coverage_cmd_for_fmt(fmt, out) - try: - run_cmd(cmd, method="run_fg") + try: + cmd = coverage_cmd_for_fmt(fmt, out) + run_cmd(cmd, method="run_fg") except ProcessExecutionError as exc: raise typer.Exit(code=exc.retcode or 1) from exc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/generate-coverage/scripts/run_python.py around lines 270 - 274, The call to coverage_cmd_for_fmt(fmt, out) is outside the ProcessExecutionError handler so venv/setup failures can escape; move the coverage_cmd_for_fmt(fmt, out) invocation inside the try that calls run_cmd(...) so any ProcessExecutionError thrown during command construction or venv setup is caught, and continue to re-raise as typer.Exit(code=exc.retcode or 1) from exc; update the try/except around run_cmd to include the cmd assignment (referencing coverage_cmd_for_fmt, cmd, run_cmd, ProcessExecutionError, and typer.Exit) so subprocess exit codes are preserved and propagated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/actions/generate-coverage/scripts/run_python.py:
- Around line 270-274: The call to coverage_cmd_for_fmt(fmt, out) is outside the
ProcessExecutionError handler so venv/setup failures can escape; move the
coverage_cmd_for_fmt(fmt, out) invocation inside the try that calls run_cmd(...)
so any ProcessExecutionError thrown during command construction or venv setup is
caught, and continue to re-raise as typer.Exit(code=exc.retcode or 1) from exc;
update the try/except around run_cmd to include the cmd assignment (referencing
coverage_cmd_for_fmt, cmd, run_cmd, ProcessExecutionError, and typer.Exit) so
subprocess exit codes are preserved and propagated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: efc8a03c-40c9-4b2d-91d9-24619bf1012b
📒 Files selected for processing (2)
.github/actions/generate-coverage/scripts/run_python.py.github/actions/generate-coverage/tests/test_scripts.py
|
@coderabbitai please create a new GitHub issue for the following: Verify each finding against the current code and only fix it if needed. In @.github/actions/generate-coverage/scripts/run_python.py around lines 270 - |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me verify the current code before creating the issue. 🏁 Script executed: Length of output: 3348 The finding is valid. 🏁 Script executed: Length of output: 52
In The one-line fix is to move the try:
cmd = coverage_cmd_for_fmt(fmt, out)
run_cmd(cmd, method="run_fg")
except ProcessExecutionError as exc:
raise typer.Exit(code=exc.retcode or 1) from exc |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Expand run_python docstrings and integration coverage, and add a developer guide for the coverage venv architecture.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Wrap developer guide tables and prose to satisfy markdownlint style checks.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/generate-coverage/scripts/run_python.py (1)
301-308:⚠️ Potential issue | 🟠 MajorMove venv bootstrap inside the
tryblock.Line 304 constructs the command before the handler runs.
coverage_cmd_for_fmt()now bootstraps.venv-coveragethrough_coverage_python_cmd(), souv venv/uv sync/uv pip installfailures, plus the post-createRuntimeError, escape as raw exceptions instead of the normal Typer exit path.Patch
- cmd = coverage_cmd_for_fmt(fmt, out) try: + cmd = coverage_cmd_for_fmt(fmt, out) run_cmd(cmd, method="run_fg") + except RuntimeError as exc: + typer.echo(str(exc), err=True) + raise typer.Exit(code=1) from exc except ProcessExecutionError as exc: raise typer.Exit(code=exc.retcode or 1) from exc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/generate-coverage/scripts/run_python.py around lines 301 - 308, The call to coverage_cmd_for_fmt (which invokes _coverage_python_cmd and bootstraps .venv-coverage) happens outside the try/except so bootstrap failures raise raw exceptions; move the construction of cmd = coverage_cmd_for_fmt(fmt, out) inside the try block before run_cmd so any errors from _coverage_python_cmd/venv creation are caught and re-raised as typer.Exit (existing except ProcessExecutionError handling around run_cmd will then handle bootstrap failures consistently).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/developers-guide.md`:
- Around line 33-35: Update the documentation examples to explicitly show the
interpreter argument for the uv commands so they match the implementation:
change the examples for `_ensure_coverage_venv()` from `uv sync --inexact
--python` and `uv pip install --python slipcover pytest coverage` to include a
placeholder interpreter like `uv sync --inexact --python <venv_python>` and `uv
pip install --python <venv_python> slipcover pytest coverage`, and ensure
references to `_coverage_python_cmd()` and `_ensure_coverage_venv()` reflect the
same `<venv_python>` placeholder.
- Around line 52-54: Update the wording in the concurrency section of the docs:
change “synchronisation” to “synchronization” so the sentence referencing
run_python.py and the `@lru_cache`(maxsize=1) on _coverage_python_cmd() uses the
repository's preferred spelling; ensure only the single word is replaced and
preserve the surrounding sentence and punctuation.
- Around line 40-44: The docs table documents main(..., baseline) but the actual
function signature uses baseline_file; update the API table so the entry for
`main` reads `(output_path, lang, fmt, github_output, baseline_file)` to match
the implementation and avoid a stale parameter name, ensuring callers copy the
correct parameter (`baseline_file`) used by the `main` function.
---
Outside diff comments:
In @.github/actions/generate-coverage/scripts/run_python.py:
- Around line 301-308: The call to coverage_cmd_for_fmt (which invokes
_coverage_python_cmd and bootstraps .venv-coverage) happens outside the
try/except so bootstrap failures raise raw exceptions; move the construction of
cmd = coverage_cmd_for_fmt(fmt, out) inside the try block before run_cmd so any
errors from _coverage_python_cmd/venv creation are caught and re-raised as
typer.Exit (existing except ProcessExecutionError handling around run_cmd will
then handle bootstrap failures consistently).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a5d4a7fd-d69c-4097-8bc0-bb1cc9862d69
📒 Files selected for processing (3)
.github/actions/generate-coverage/scripts/run_python.py.github/actions/generate-coverage/tests/test_scripts.pydocs/developers-guide.md
Move coverage command construction under main error handling and refresh developer guide details for uv interpreter arguments.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. .github/actions/generate-coverage/scripts/run_python.py Comment on lines +275 to +301 """Run slipcover coverage and write the result to ``GITHUB_OUTPUT``.
Parameters
----------
output_path : Path
Destination path for the coverage output file.
lang : str
Detected project language (``"rust"``, ``"python"``, or
``"mixed"``). When ``"mixed"``, the output file is renamed to
include a ``.python`` infix.
fmt : str
Coverage format identifier passed to :func:`coverage_cmd_for_fmt`.
github_output : Path
Path to the ``GITHUB_OUTPUT`` append file where ``file=`` and
``percent=`` are written.
baseline_file : Path or None
Optional path to a previous coverage baseline file. When present,
the previous percentage is echoed to the log.
Raises
------
typer.Exit
With the subprocess return code when the slipcover/coverage command
exits non-zero, or when ``coverage xml`` fails in ``coveragepy``
format mode.
"""
out = _resolve_output_path(output_path, lang)❌ New issue: Complex Method |
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. .github/actions/generate-coverage/scripts/run_python.py Comment on file import collections.abc as cabc # noqa: TC003 - used at runtime
import contextlib
import os❌ New issue: Overall Code Complexity |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 21020
Status per check (validated against the current code):
AI coding agent prompt (apply all changes): Suppression messages for CodeScene (paste into diagnostic suppression): Unit Architecture The function _coverage_python_cmd() uses functools.lru_cache(maxsize=1), which is Python’s idiomatic, standard-library mechanism for lazy, once-per-process initialisation. Its docstring explicitly declares the side-effect (“creating it on first use”), and all I/O (uv venv/sync/pip) resides in _ensure_coverage_venv(), not in the accessor itself. For this single-threaded GitHub Actions step, splitting setup into a separate function that all call sites must remember to invoke would duplicate responsibilities, increase cyclomatic complexity across the module, and risk improper sequencing. The present design is intentional, minimal, and documented in docs/developers-guide.md. Domain Architecture run_python.py is internal CI scripting, not a reusable library. Introducing an adapter layer (e.g., a VenvAdapter class, injected executors/FS, custom domain errors) would add indirection and maintenance overhead without user-facing benefit, and would complicate tests already well served by monkeypatches. Keeping infrastructure calls inline (typer.echo, run_cmd, filesystem ops) follows the repository’s scripting standards and keeps the action readable and low risk. If you want me to push these changes as a follow-up commit, say the word and I will generate the ready-to-apply patch. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/generate-coverage/scripts/run_python.py:
- Around line 101-106: The helper _find_coverage_python currently returns a
potentially relative interpreter path; update it to resolve the interpreter to
an absolute path (e.g., using Path(...).resolve() or os.path.abspath) before
returning and cache that resolved value so subsequent calls return the same
absolute path; apply the same change at the other return sites referenced (the
code paths around where the local variable python is set and returned) and
ensure the raised RuntimeError still references COVERAGE_VENV but the returned
value always satisfies the documented absolute-path contract.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e1af622b-40a6-4578-86bf-1571c711a7aa
📒 Files selected for processing (2)
.github/actions/generate-coverage/scripts/run_python.pydocs/developers-guide.md
| python = _find_coverage_python() | ||
| if python is None: | ||
| msg = f"Coverage venv Python executable not found in {COVERAGE_VENV}" | ||
| raise RuntimeError(msg) | ||
| return python | ||
|
|
There was a problem hiding this comment.
Return a resolved interpreter path to satisfy the absolute-path contract.
Align implementation with the documented return contract. Line 101 and Line 160 currently propagate a relative interpreter path, while the docstrings state an absolute path. Resolve once before returning and cache the resolved value.
🔧 Proposed fix
def _recreate_coverage_venv() -> Path:
@@
- return python
+ return python.resolve()
@@
def _ensure_coverage_venv() -> str:
@@
python = _find_coverage_python()
if python is None:
python = _recreate_coverage_venv()
+ else:
+ python = python.resolve()
@@
- return str(python)
+ return str(python)Also applies to: 117-121, 160-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/generate-coverage/scripts/run_python.py around lines 101 -
106, The helper _find_coverage_python currently returns a potentially relative
interpreter path; update it to resolve the interpreter to an absolute path
(e.g., using Path(...).resolve() or os.path.abspath) before returning and cache
that resolved value so subsequent calls return the same absolute path; apply the
same change at the other return sites referenced (the code paths around where
the local variable python is set and returned) and ensure the raised
RuntimeError still references COVERAGE_VENV but the returned value always
satisfies the documented absolute-path contract.
Split slipcover execution and coverage percent parsing out of main to lower cyclomatic complexity.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Replace separate slipcover and percent parsing helpers with a single _run_coverage helper to reduce complexity metrics.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Log coverage venv reuse and uv sync success, document run_script non-raising behavior, and cover previous baseline reporting.
There was a problem hiding this comment.
Gates Failed
(1 hotspot with Complex Method)
(1 file with Complex Method)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| run_python.py | 1 rule in this hotspot | 9.69 → 9.39 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| run_python.py | 1 advisory rule | 9.69 → 9.39 | Suppress |
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
| def _ensure_coverage_venv() -> str: | ||
| """Create or repair the coverage venv and install project/test tooling. | ||
|
|
||
| Checks whether ``.venv-coverage`` contains a healthy Python executable. | ||
| If not, delegates to :func:`_recreate_coverage_venv` to remove any | ||
| broken state and create a fresh venv. Then runs ``uv sync`` to install | ||
| project dependencies, followed by ``uv pip install`` to add | ||
| ``slipcover``, ``pytest``, and ``coverage``. | ||
|
|
||
| Returns | ||
| ------- | ||
| str | ||
| Absolute path to the Python executable inside the coverage venv. | ||
|
|
||
| Raises | ||
| ------ | ||
| RuntimeError | ||
| Propagated from :func:`_recreate_coverage_venv` when the Python | ||
| executable cannot be located after venv creation. | ||
| plumbum.commands.processes.ProcessExecutionError | ||
| Propagated from ``uv sync`` or ``uv pip install`` when either | ||
| command exits with a non-zero return code. | ||
| """ | ||
| python = _find_coverage_python() | ||
| if python is None: | ||
| python = _recreate_coverage_venv() | ||
| else: | ||
| typer.echo(f"Reusing existing coverage venv at {COVERAGE_VENV}") | ||
| typer.echo(f"Installing project dependencies into {COVERAGE_VENV}") | ||
| previous_project_environment = os.environ.get("UV_PROJECT_ENVIRONMENT") | ||
| os.environ["UV_PROJECT_ENVIRONMENT"] = str(COVERAGE_VENV.resolve()) | ||
| try: | ||
| run_cmd(uv[*PROJECT_SYNC_ARGS, str(python)]) | ||
| typer.echo(f"Project dependencies installed into {COVERAGE_VENV}") | ||
| except ProcessExecutionError as exc: | ||
| typer.echo( | ||
| f"uv sync failed with code {exc.retcode}: {exc.stderr}", | ||
| err=True, | ||
| ) | ||
| raise | ||
| finally: | ||
| if previous_project_environment is None: | ||
| os.environ.pop("UV_PROJECT_ENVIRONMENT", None) | ||
| else: | ||
| os.environ["UV_PROJECT_ENVIRONMENT"] = previous_project_environment | ||
| typer.echo(f"Installing coverage tooling {TOOLING_PACKAGES} into {COVERAGE_VENV}") | ||
| try: | ||
| run_cmd(uv["pip", "install", "--python", str(python), *TOOLING_PACKAGES]) | ||
| except ProcessExecutionError as exc: | ||
| typer.echo( | ||
| f"uv pip install failed with code {exc.retcode}: {exc.stderr}", | ||
| err=True, | ||
| ) | ||
| raise | ||
| typer.echo(f"Coverage tooling installed into {COVERAGE_VENV}") | ||
| return str(python) |
There was a problem hiding this comment.
❌ New issue: Complex Method
_ensure_coverage_venv has a cyclomatic complexity of 9, threshold = 9
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/actions/generate-coverage/scripts/run_python.py (1)
101-106:⚠️ Potential issue | 🟡 MinorReturn absolute interpreter paths or fix the docstrings immediately.
Align behaviour with contract. Line 105 and Line 160 currently return potentially relative paths, while the docstrings promise absolute paths. Resolve the path before returning, or rewrite the docstrings to state relative paths.
Patch
def _recreate_coverage_venv() -> Path: @@ - return python + return python.resolve() def _ensure_coverage_venv() -> str: @@ - return str(python) + return str(python.resolve())Also applies to: 160-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/generate-coverage/scripts/run_python.py around lines 101 - 106, The function that returns the coverage venv Python (the value coming from _find_coverage_python()) is returning a potentially relative path despite the docstring promising an absolute path; call os.path.abspath (or pathlib.Path(...).resolve(strict=False)) on the path returned by _find_coverage_python() (and any similar return at the other location around the second occurrence) before returning, and ensure the returned value is stored back to the same symbol (e.g., python) so callers get an absolute path; alternatively, if you prefer relative paths, update the function docstrings to state they may return relative paths instead of changing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/actions/generate-coverage/scripts/run_python.py:
- Around line 101-106: The function that returns the coverage venv Python (the
value coming from _find_coverage_python()) is returning a potentially relative
path despite the docstring promising an absolute path; call os.path.abspath (or
pathlib.Path(...).resolve(strict=False)) on the path returned by
_find_coverage_python() (and any similar return at the other location around the
second occurrence) before returning, and ensure the returned value is stored
back to the same symbol (e.g., python) so callers get an absolute path;
alternatively, if you prefer relative paths, update the function docstrings to
state they may return relative paths instead of changing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 529c0307-ad4b-4681-902c-3ff896178bc2
📒 Files selected for processing (1)
.github/actions/generate-coverage/scripts/run_python.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/generate-coverage/tests/test_scripts.py:
- Around line 1768-1770: _tests/test_scripts._coverage_python currently returns
COVERAGE_VENV / "bin" / "python" unconditionally which diverges from
run_python._find_coverage_python; update _coverage_python to mirror the
production candidate logic by checking the platform (sys.platform or os.name)
and returning COVERAGE_VENV / "Scripts" / "python.exe" on Windows and
COVERAGE_VENV / "bin" / "python" otherwise so tests will match
run_python._find_coverage_python's behavior.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 707ce737-6c29-45d2-9c78-0e931ca286c2
📒 Files selected for processing (2)
.github/actions/generate-coverage/scripts/run_python.py.github/actions/generate-coverage/tests/test_scripts.py
| def _coverage_python(run_python_module: ModuleType) -> str: | ||
| """Return the expected throwaway venv Python path.""" | ||
| return str(run_python_module.COVERAGE_VENV / "bin" / "python") |
There was a problem hiding this comment.
Hardcoded POSIX path diverges from production code.
_coverage_python() returns "bin/python" unconditionally, whereas run_python._find_coverage_python() probes both bin/python and Scripts/python.exe. If unit tests ever run on Windows without the integration-test skip marker, assertions will fail. Mirror the production candidate logic or parametrise the helper.
Proposed fix
def _coverage_python(run_python_module: ModuleType) -> str:
"""Return the expected throwaway venv Python path."""
- return str(run_python_module.COVERAGE_VENV / "bin" / "python")
+ import sys
+ if sys.platform == "win32":
+ return str(run_python_module.COVERAGE_VENV / "Scripts" / "python.exe")
+ return str(run_python_module.COVERAGE_VENV / "bin" / "python")🤖 Prompt for AI Agents
In .github/actions/generate-coverage/tests/test_scripts.py at lines 1768-1770,
_coverage_python() always returns a POSIX-style path. Update the helper to
detect sys.platform and return COVERAGE_VENV / "Scripts" / "python.exe" on
Windows and COVERAGE_VENV / "bin" / "python" on other platforms, matching the
candidate logic in run_python._find_coverage_python().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/generate-coverage/tests/test_scripts.py around lines 1768 -
1770, _tests/test_scripts._coverage_python currently returns COVERAGE_VENV /
"bin" / "python" unconditionally which diverges from
run_python._find_coverage_python; update _coverage_python to mirror the
production candidate logic by checking the platform (sys.platform or os.name)
and returning COVERAGE_VENV / "Scripts" / "python.exe" on Windows and
COVERAGE_VENV / "bin" / "python" otherwise so tests will match
run_python._find_coverage_python's behavior.
1 participant
Summary
This branch runs the
generate-coveragePython coverage phase from a short-lived virtual environment instead of relying onuv runor any system interpreter install path. The change avoids PEP 668 failures on GitHub-hosted Ubuntu runners where Python 3.12 is externally managed, while keeping coverage tooling isolated from the system interpreter.It also hardens the Makefile check targets so automation with a narrow
PATHcan still finduvandaction-validatorfrom their common user-local install locations.Review walkthrough
.venv-coveragecreation,uv pip install --pythontooling install, and venv Python reuse for slipcover, pytest, and coverage XML export.--systeminstalls.UVandACTION_VALIDATORcommand resolution used by local and hook-driven checks.Validation
python -c "import ast, sys; ast.parse(open('.github/actions/generate-coverage/scripts/run_python.py').read()); print('parse ok')": passeduv run --with typer --with packaging --with plumbum --with pyyaml pytest .github/actions/generate-coverage/tests/test_scripts.py -k 'coverage_cmd_uses_venv_python or tmp_coveragepy_xml_invokes_venv_python or run_python_cobertura_passes_out_flag' -v: passed, 3 selected tests passedmake check-fmt: passedmake typecheck: passedmake lint: passedmake test: passed, 668 passed and 90 skippedmake check-fmt lint typecheck: passedenv PATH=/usr/bin:/bin make check-fmt lint typecheck: passedNotes
The branch deliberately does not add
--break-system-packagesor restore a system install path. Coverage tooling is installed into.venv-coverageand invoked through that venv's Python interpreter.Summary by Sourcery
Run Python coverage tooling from an isolated, job-local virtual environment instead of via
uv run, and make local tooling resolution in the Makefile more robust.New Features:
.venv-coveragevirtual environment and helper functions to create it, install coverage tooling into it, and reuse its Python interpreter for all coverage commands.Bug Fixes:
uv run --withfor coverage, preventing failures on externally managed Python installations such as GitHub-hosted Ubuntu runners.Enhancements:
uv,action-validator, andmarkdownlintfrom common user-local install locations and to lint only changed Markdown files where possible.Documentation: