Enforce Ruff DOC rules and numpy docstrings - #234
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe pull request extracts backend, pipeline, stream, context, tracing, and test-support modules; adds validation and ratio helpers; expands docstrings; updates Ruff documentation linting; and adds regression coverage for pipeline, stream, context, and validation behaviour. ChangesRuntime and contract updates
Sequence Diagram(s)sequenceDiagram
participant TeeProfileWorker
participant BackendSelector
participant CuprumBackend
participant Pipeline
participant StreamPump
TeeProfileWorker->>BackendSelector: activate selected backend
BackendSelector->>CuprumBackend: clear backend caches
Pipeline->>StreamPump: relay stage output
StreamPump-->>Pipeline: return stream completion
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Sorry @leynos, your pull request is larger than the review limit of 150000 diff characters
|
@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. benchmarks/_benchmark_type_validators.py Comment on lines +43 to +54 """Validate integer values and reject booleans.
Returns
-------
int
The validated integer value.
Raises
------
TypeError
If ``value`` is not an ``int`` or is a ``bool``.
"""❌ New issue: Code Duplication |
|
@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. Comment on lines +13 to +24 """Validate and return a mapping-like JSON object.
Returns
-------
cabc.Mapping[str, object]
The validated mapping.
Raises
------
TypeError
If *value* is not a mapping.
"""❌ New issue: Code Duplication |
|
@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. benchmarks/fetch_main_benchmark_baseline.py Comment on lines +54 to +65 """Validate that *value* is an integer.
Returns
-------
int
The value narrowed to ``int``.
Raises
------
TypeError
If *value* is a boolean or not an integer.
"""❌ New issue: Code Duplication |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
@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. benchmarks/tee_profile_profilers.py Comment on lines +277 to +283 """Execute the scenario under Linux perf.
Returns
-------
cabc.Mapping[str, object]
The worker result payload recorded under perf.
"""❌ New issue: Code Duplication |
|
@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. cuprum/_subprocess_execution.py Comment on lines +267 to +280 """Execute a subprocess and return the command result.
Returns
-------
CommandResult
Structured information about the completed process.
Raises
------
TimeoutError
If the process exceeds the configured timeout.
asyncio.CancelledError
If the surrounding task is cancelled while awaiting the process.
"""❌ New issue: Large 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. 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. cuprum/unittests/test_build_final_results_property.py Comment on lines +90 to +96 """Return whether every failure index addresses the compacted result list.
Returns
-------
bool
True when every failure index falls within the result list bounds.
"""❌ New issue: Code Duplication |
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.
This comment was marked as resolved.
This comment was marked as resolved.
|
@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. cuprum/unittests/test_stream_pump_runtime_behaviour.py Comment on lines +32 to +38 """Return the next queued chunk, or empty bytes at EOF.
Returns
-------
bytes
The next queued chunk, or empty bytes once exhausted.
"""❌ New issue: Code Duplication |
|
@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. tests/behaviour/test_benchmark_comparison_report_behaviour.py Comment on lines +155 to +162 """Create valid benchmark comparison inputs.
Returns
-------
FixtureBundle
The prepared artefact paths for a valid comparison run.
"""❌ New issue: Code Duplication |
|
@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. tests/behaviour/test_concurrency_helper.py Comment on lines +178 to +185 """Execute commands concurrently with default settings.
Returns
-------
_ConcurrentExecution
The concurrent result and its measured elapsed time.
"""❌ New issue: Code Duplication |
This comment was marked as resolved.
This comment was marked as resolved.
|
@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. tests/behaviour/test_context_hooks.py Comment on lines +170 to +176 """Create context with multiple before hooks for ordering test.
Returns
-------
CuprumContext
A context registering the ordered before hooks.
"""❌ New issue: Code Duplication |
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.
|
@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. tests/behaviour/test_execution_runtime.py Comment on lines +183 to +189 """Construct a SafeCmd that blocks until cancelled.
Returns
-------
dict[str, object]
The worker command and its supporting fixture state.
"""❌ New issue: Code Duplication |
|
@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. tests/behaviour/test_pipeline_execution.py Comment on lines +163 to +169 """Create a three stage pipeline where the first stage exits non-zero.
Returns
-------
_ScenarioPipeline
The pipeline under test with its allowlist.
"""❌ New issue: Code Duplication |
|
@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. cuprum/unittests/test_pipeline.py Comment on file async def read(self, _: int) -> bytes:
"""Return the next queued chunk, or empty bytes when exhausted."""
"""Return the next queued chunk, or empty bytes when exhausted.❌ New issue: Low Cohesion |
Document parameters and propagated exceptions, harden concurrent-config validation, modernise hook merging, and split an oversized test module. - Add NumPy Parameters/Raises sections to public helpers across benchmarks/ and cuprum/ (catalogue, sh, builders, concurrent, streams); propagated exceptions carry a scoped ``# noqa: DOC502`` with a reason. - Restore Raises sections in cuprum/_streams_rs.py alongside the Notes prose. - ConcurrentConfig.__post_init__ rejects bool/non-int concurrency; ConcurrentResult.__post_init__ validates failures are in-range, unique, and strictly ascending. - _merge_hooks uses a PEP 695 function-scoped type parameter; drop the module-level TypeVar. - Test hygiene: assert-with-message, slots=True dataclass, narrowed exception suppression, AfterHook casts, Oxford "parameterized" spelling. - Split test_pipeline_stream_backend_selection.py into _pump_stream_dispatch_support.py + two focused test modules under the 400-line limit; add a *_support.py per-file-ignore; regenerate the maturin wheel-manifest snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the one-line docstring with a NumPy-style docstring including a Returns section: it returns None and does not suppress exceptions from the with body. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove structured Returns/Raises sections from private helpers, nested helpers, and test stubs whose one-line summary fully communicates purpose, relying on the ignore-one-line-docstrings exemption: - test_concurrency.py: nested make_tracker helpers and the async exercise - test_stream_pump_runtime_behaviour.py: stub read/drain methods and the seven nested exercise helpers - test_pipeline_process_lifecycle.py: _StubSpawnProcess.wait and the nested fake_create_subprocess_exec spawn stub - stream_pipes.py: _join_or_raise and the nested pump callback - test_pipeline_execution.py: _build_scenario_pipeline_from_commands Left intact where a structured section is required or documents a genuine invariant: _drain_blocking_payload_size (pipe-buffer rationale + required Returns), _join_or_raise's siblings feed_source_pipe/pump_payload_through_pipes (public helpers with concurrency invariants and required Yields/Returns), _run_hook_test/_make_test_pipeline (documented private helpers), and the rust-streams test functions that directly raise OSError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- _create_worker_command: replace the multi-line docstring (and its Returns section) with a single-line summary; runtime behaviour unchanged. - _generate_test_data: drop the structured Returns section and collapse the docstring to one line, moving the deterministic-local-RNG / no-global-state rationale to an inline comment at the RNG construction site so DOC201 stays satisfied and the invariant is documented where it applies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Correct "serialised" to "serialized" in the Returns section; docs-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/developers-guide.md (1)
527-528: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument non-finite timeout rejection.
State that
_validate_timeoutrejects non-finite values, includingNaNand infinities, as well as negative values. The helper incuprum/context/_policy.pycallsmath.isfinite()before the negative check, so the guide currently understates the contract.Triage:
[type:docstyle]As per coding guidelines, keep developer documentation aligned with the implementation’s actual contract.
🤖 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 `@docs/developers-guide.md` around lines 527 - 528, Update the `_validate_timeout` documentation in the developers guide to state that it rejects non-finite values, including NaN and positive or negative infinity, in addition to negative values; retain the existing description of float coercion and None preservation.Source: Coding guidelines
cuprum/sh.py (1)
280-294: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the encoding failure.
Add a NumPy-style
Raisessection forUnicodeEncodeError.resolve()still callsself.text.encode(ctx.encoding, ctx.errors)at Line [296], so strict encoding failures remain observable; removing that section hides a real failure mode from callers.As per coding guidelines, public interfaces require full structured documentation of their behaviour and failure modes.
🤖 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 `@cuprum/sh.py` around lines 280 - 294, Add a NumPy-style Raises section to the public resolve method documentation, declaring UnicodeEncodeError for failures from encoding self.text with ctx.encoding and ctx.errors. Keep the existing return and parameter documentation unchanged.Source: Coding guidelines
tests/helpers/maturin_pins.py (1)
69-103: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReturn a typed structure for the synchronized pins.
Define a
TypedDictor@dataclass(slots=True)for the three fixed pin fields instead of returningdict[str, str]. This lets Pyright detect missing or renamed sources rather than permitting arbitrary keys.As per coding guidelines, use
TypedDictor@dataclass(slots=True)for structured data instead of a plain dictionary.🤖 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/helpers/maturin_pins.py` around lines 69 - 103, Define a typed structure for the three fixed pin entries returned by read_maturin_pins, using a TypedDict or slots dataclass. Update read_maturin_pins and its return annotation to use that structure while preserving the existing source keys and pin values, so static checking rejects missing or renamed fields.Source: Coding guidelines
🤖 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 `@cuprum/_concurrent_config.py`:
- Around line 109-123: Replace the multi-line docstring on the private
__post_init__ hook with the single summary line “Validate result indexes and the
submission mapping.” Keep the detailed documentation on ConcurrentResult
unchanged, and preserve only any necessary non-obvious rationale as an inline
comment.
- Around line 60-67: In the concurrency validation within the configuration
class, change the exception for values rejected by the type check to TypeError
while retaining ValueError for integer values below one. Update the public
configuration contract documentation to describe the TypeError raised for
non-integer and bool inputs.
- Around line 124-138: Update the failure-validation loop in the concurrent
configuration initializer to reject any value whose exact type is not int,
thereby excluding bool, before performing range or ordering checks. Raise
TypeError with the invalid type name, and add regression tests covering
non-integer and boolean failure indexes.
In `@cuprum/unittests/_pump_stream_dispatch_support.py`:
- Around line 91-104: Deduplicate the repeated backend reset sequence in the
clear_backend_caches fixture by extracting it into a local helper, then invoke
that helper both before and after yield. Keep the helper’s operations unchanged,
including availability reset, dispatch reset, and both backend cache clears.
In `@cuprum/unittests/test_pump_stream_dispatch_selection.py`:
- Around line 79-85: Update both fake_pump stand-ins in the pump stream dispatch
tests to explicitly discard their unused reader and writer parameters, following
the existing convention used by _fake_python_fallback and the FD-blocking test’s
inline stubs.
---
Outside diff comments:
In `@cuprum/sh.py`:
- Around line 280-294: Add a NumPy-style Raises section to the public resolve
method documentation, declaring UnicodeEncodeError for failures from encoding
self.text with ctx.encoding and ctx.errors. Keep the existing return and
parameter documentation unchanged.
In `@docs/developers-guide.md`:
- Around line 527-528: Update the `_validate_timeout` documentation in the
developers guide to state that it rejects non-finite values, including NaN and
positive or negative infinity, in addition to negative values; retain the
existing description of float coercion and None preservation.
In `@tests/helpers/maturin_pins.py`:
- Around line 69-103: Define a typed structure for the three fixed pin entries
returned by read_maturin_pins, using a TypedDict or slots dataclass. Update
read_maturin_pins and its return annotation to use that structure while
preserving the existing source keys and pin values, so static checking rejects
missing or renamed fields.
🪄 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: f20ef573-9ecf-42a1-98b4-10f2b9ef3067
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
benchmarks/benchmark_profile.pybenchmarks/ci_benchmark_ratchet_profile.pybenchmarks/comparison_analysis.pybenchmarks/ratchet_rust_performance.pybenchmarks/sinks.pybenchmarks/tee_profile_scenarios.pycuprum/_concurrent_config.pycuprum/_process_lifecycle.pycuprum/_streams.pycuprum/_streams_rs.pycuprum/builders/args.pycuprum/builders/git.pycuprum/catalogue.pycuprum/concurrent.pycuprum/context/_policy.pycuprum/sh.pycuprum/unittests/__snapshots__/test_maturin_build.ambrcuprum/unittests/_pump_stream_dispatch_support.pycuprum/unittests/_tee_profile_concurrency_support.pycuprum/unittests/conftest.pycuprum/unittests/strategies.pycuprum/unittests/test_context.pycuprum/unittests/test_env_context.pycuprum/unittests/test_fixture_generation.pycuprum/unittests/test_pipeline_wait.pycuprum/unittests/test_pump_stream_dispatch_fd_blocking.pycuprum/unittests/test_pump_stream_dispatch_selection.pycuprum/unittests/test_safe_cmd_context.pycuprum/unittests/test_safe_cmd_run.pycuprum/unittests/test_safe_cmd_stdin.pycuprum/unittests/test_stream_parity.pycuprum/unittests/test_tee_profile_worker_selector_metrics.pycuprum/unittests/test_validation.pydocs/developers-guide.mdpyproject.tomlscripts/tests/test_typos_rollout.pytests/behaviour/_context_hooks_support.pytests/behaviour/test_backend_dispatcher_behaviour.pytests/behaviour/test_benchmark_ci_ratchet_behaviour.pytests/behaviour/test_benchmark_comparison_report_behaviour.pytests/helpers/maturin.pytests/helpers/maturin_pins.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/shared-actions(auto-detected)leynos/pylint-pypy-shim(auto-detected)leynos/whitaker(auto-detected)
|
@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. Complex Methodcuprum/_concurrent_config.py: ConcurrentResult.post_init What lead to degradation?ConcurrentResult.post_init has a cyclomatic complexity of 9, threshold = 9 Why does this problem occur?A Complex Method has a high cyclomatic complexity. The recommended threshold for the Python language is a cyclomatic complexity lower than 9. How to fix it?There are many reasons for Complex Method. Sometimes, another design approach is beneficial such as a) modeling state using an explicit state machine rather than conditionals, or b) using table lookup rather than long chains of logic. In other scenarios, the function can be split using EXTRACT FUNCTION. Just make sure you extract natural and cohesive functions. Complex Methods can also be addressed by identifying complex conditional expressions and then using the DECOMPOSE CONDITIONAL refactoring. Helpful refactoring examplesTo get a general understanding of what this code health issue looks like - and how it might be addressed - we have prepared some diffs for illustrative purposes. SAMPLE# complex_method.js
function postItem(item) {
if (!item.id) {
- if (item.x != null && item.y != null) {
- post(item);
- } else {
- throw Error("Item must have x and y");
- }
+ // extract a separate function for creating new item
+ postNew(item);
} else {
- if (item.x < 10 && item.y > 25) {
- put(item);
- } else {
- throw Error("Item must have an x and y value between 10 and 25");
- }
+ // and one for updating existing items
+ updateItem(item);
}
}
+
+function postNew(item) {
+ validateNew(item);
+ post(item);
+}
+
+function updateItem(item) {
+ validateUpdate(item);
+ put(item);
+}
+ |
This comment was marked as resolved.
This comment was marked as resolved.
Code: - ConcurrentConfig.__post_init__ raises TypeError (was ValueError) for non-int/bool concurrency; ValueError retained for values below one. The concurrency attribute contract now documents this. - ConcurrentResult.__post_init__ rejects any failures element whose exact type is not int (excluding bool) with TypeError before the range/ordering checks; docstring collapsed to a single summary line with rationale kept as inline comments. - _pump_stream_dispatch_support: deduplicate the backend reset sequence in clear_backend_caches via a local reset() helper called before and after yield. - test_pump_stream_dispatch_selection: both fake_pump stubs discard unused reader/writer via ``del reader, writer``. - maturin_pins: read_maturin_pins returns a functional TypedDict (MaturinPins) so static checking rejects missing/renamed pin entries. Tests: - Add regression tests for bool/non-int concurrency and bool/non-int failure indexes raising TypeError. Docs: - StdinInput.resolve documents the propagated UnicodeEncodeError (Raises + scoped DOC502 suppression). - developers-guide: _validate_timeout now documents non-finite rejection (NaN, +/- infinity). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CodeScene complex-method remediation: ConcurrentResult.__post_init__ validated two independent invariants inline. Split them into two private module-level helpers so the initializer is orchestration only, with no change to accepted values or exception text. - Add _validate_failure_indices(failures, *, result_count): the former ``previous = -1`` loop (in-range + strictly-ascending/unique, plus the exact-int guard), with identical ValueError/TypeError messages. - Add _resolve_submission_indices(submission_indices, *, result_count): None backfills tuple(range(result_count)) (empty when result_count == 0); a supplied same-length tuple is returned unchanged; a length mismatch raises the existing ValueError. __post_init__ now always assigns the resolved tuple via object.__setattr__. Both helpers stay private (no export surface change); collect-all and fail-fast assembly in concurrent.py are untouched. Tests: add focused direct coverage for empty-results identity backfill, out-of-range, duplicate, and descending failure indices (the existing identity-backfill, explicit-empty rejection, length-mismatch, and compacted fail-fast mapping tests are retained). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
cuprum/sh.py (1)
660-667: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove redundant nested-helper
Returnssections.Keep the concise summaries; neither helper documents a non-obvious invariant,
boundary, trade-off, concurrency property, or failure mode requiring structured
documentation.
cuprum/sh.py#L660-L667: Remove theReturnsblock from nestedbuilder().cuprum/unittests/test_safe_cmd_run.py#L342-L348: Remove theReturnsblock
from nestedorchestrate().Based on learnings, private helpers should avoid redundant structured sections
unless narrative explains a non-obvious guarantee. As per path instructions,
private functions and methods require a single-line summary.🤖 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 `@cuprum/sh.py` around lines 660 - 667, Remove the redundant Returns sections while preserving the concise one-line summaries for nested helper functions: delete the structured Returns block from builder in cuprum/sh.py at lines 660-667 and from orchestrate in cuprum/unittests/test_safe_cmd_run.py at lines 342-348.Sources: Path instructions, Learnings
cuprum/_streams_rs.py (1)
126-130: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove
NotesafterRaises. Keep the implementation note there and leave the exception summary inRaises; NumPy order putsParameters/RaisesbeforeNotes, and this layout breaks that rule.🤖 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 `@cuprum/_streams_rs.py` around lines 126 - 130, Reorder the docstring sections in the relevant stream implementation so the Notes section follows Raises, preserving the existing implementation note content and keeping the exception summary under Raises.Sources: Coding guidelines, MCP tools
cuprum/unittests/test_safe_cmd_stdin.py (1)
45-52: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the injected fixture parameter.
Add a NumPy-style
Parameterssection forrequest : pytest.FixtureRequest; it selects the asynchronous or synchronous execution strategy returned by this fixture.🤖 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 `@cuprum/unittests/test_safe_cmd_stdin.py` around lines 45 - 52, Document the injected request parameter in execution_strategy by adding a NumPy-style Parameters section for request: pytest.FixtureRequest, describing that it selects the asynchronous or synchronous execution strategy returned by the fixture.Sources: Coding guidelines, Path instructions
benchmarks/benchmark_profile.py (1)
102-106: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winComplete the propagated exception contracts.
The new public docstrings document only selected failure branches, while the implementations also propagate parsing, structural-validation, strict-zip, subprocess, malformed-record, or incomplete-group failures.
benchmarks/benchmark_profile.py#L102-L106: document missing and malformed profile metadata.benchmarks/ci_benchmark_ratchet_profile.py#L43-L46: document file, JSON, and structural-validation failures.benchmarks/ci_benchmark_ratchet_profile.py#L113-L117: document strict-zip and field-validation failures.benchmarks/ci_benchmark_ratchet_profile.py#L229-L241: documentsubprocess.CalledProcessErrorand propagated loader/selection failures.benchmarks/comparison_analysis.py#L252-L256: document malformed payload and incomplete Python/Rust group failures.🤖 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 `@benchmarks/benchmark_profile.py` around lines 102 - 106, Complete the public docstring exception contracts for all affected sites: in benchmarks/benchmark_profile.py lines 102-106, document missing and malformed profile metadata; in benchmarks/ci_benchmark_ratchet_profile.py lines 43-46, document file, JSON, and structural-validation failures; at lines 113-117, document strict-zip and field-validation failures; and at lines 229-241, document subprocess.CalledProcessError plus propagated loader and selection failures; in benchmarks/comparison_analysis.py lines 252-256, document malformed payload and incomplete Python/Rust group failures. Anchor each update to the corresponding visible function or method and preserve the existing documented exceptions.Sources: Coding guidelines, Path instructions
tests/helpers/maturin.py (1)
219-223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the private helper docstring concise.
Replace the structured
Returnssection for_locate_dist_info_wheelwith a single-line summary. Retain only non-obvious invariants in prose, in line with the repository rule for private helpers.🤖 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/helpers/maturin.py` around lines 219 - 223, Shorten the docstring for the private helper _locate_dist_info_wheel by replacing the structured Returns section with a concise single-line summary, preserving only any non-obvious invariants required by the repository conventions.Sources: Path instructions, Learnings
cuprum/unittests/conftest.py (1)
37-52: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the
redactdocstring with the implementation.Change “mappings” to “dictionaries” (or update the implementation to accept
collections.abc.Mapping); the function only recurses throughdictandlist. State that non-container values are returned unchanged.Proposed documentation fix
- The recursively traversed value; mappings and lists are copied + The recursively traversed value; dictionaries and lists are copied recursively, while other values are returned unchanged. ... - A copy of ``obj`` with the nominated keys redacted. + A copy of container values in ``obj`` with nominated keys redacted; + non-container values are returned unchanged.🤖 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 `@cuprum/unittests/conftest.py` around lines 37 - 52, Update the redact docstring to describe the implementation accurately: replace “mappings” with “dictionaries” and clarify that only dictionary/list containers are copied recursively, while non-container values are returned unchanged. Keep the existing key-redaction behavior and parameter descriptions intact.Source: Path instructions
🤖 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 `@cuprum/unittests/test_concurrency.py`:
- Line 518: Update the assertion for ConcurrentResult submission_indices to
include a clear diagnostic message identifying the omitted-index backfill
contract, while preserving the existing expected empty tuple.
- Line 372: Replace the type-ignore suppression at
cuprum/unittests/test_concurrency.py lines 372-372 with typ.cast("int", 1.5)
when constructing ConcurrentConfig. At lines 513-513, replace the corresponding
suppression with typ.cast("int", 0.0) for the failure index, preserving both
invalid runtime-value tests without silencing Pyright.
In `@cuprum/unittests/test_pump_stream_dispatch_selection.py`:
- Around line 29-72: Define a TypedDict for the parametrized case fields
backend_env, rust_available, force_fd_extraction_failure, and
expected_rust_fd_attempts, then annotate the pytest parameter and
test_dispatch_falls_back_to_python case argument with it. Remove the typ.cast
calls and access each field directly so Pyright validates the field types.
In `@tests/behaviour/test_benchmark_ci_ratchet_behaviour.py`:
- Around line 109-119: Remove the structured Returns sections from both private
_prepare_fixture_bundle docstrings, leaving each helper with a concise
single-line docstring. Apply this change in
tests/behaviour/test_benchmark_ci_ratchet_behaviour.py lines 109-119 and
tests/behaviour/test_benchmark_comparison_report_behaviour.py lines 83-91.
---
Outside diff comments:
In `@benchmarks/benchmark_profile.py`:
- Around line 102-106: Complete the public docstring exception contracts for all
affected sites: in benchmarks/benchmark_profile.py lines 102-106, document
missing and malformed profile metadata; in
benchmarks/ci_benchmark_ratchet_profile.py lines 43-46, document file, JSON, and
structural-validation failures; at lines 113-117, document strict-zip and
field-validation failures; and at lines 229-241, document
subprocess.CalledProcessError plus propagated loader and selection failures; in
benchmarks/comparison_analysis.py lines 252-256, document malformed payload and
incomplete Python/Rust group failures. Anchor each update to the corresponding
visible function or method and preserve the existing documented exceptions.
In `@cuprum/_streams_rs.py`:
- Around line 126-130: Reorder the docstring sections in the relevant stream
implementation so the Notes section follows Raises, preserving the existing
implementation note content and keeping the exception summary under Raises.
In `@cuprum/sh.py`:
- Around line 660-667: Remove the redundant Returns sections while preserving
the concise one-line summaries for nested helper functions: delete the
structured Returns block from builder in cuprum/sh.py at lines 660-667 and from
orchestrate in cuprum/unittests/test_safe_cmd_run.py at lines 342-348.
In `@cuprum/unittests/conftest.py`:
- Around line 37-52: Update the redact docstring to describe the implementation
accurately: replace “mappings” with “dictionaries” and clarify that only
dictionary/list containers are copied recursively, while non-container values
are returned unchanged. Keep the existing key-redaction behavior and parameter
descriptions intact.
In `@cuprum/unittests/test_safe_cmd_stdin.py`:
- Around line 45-52: Document the injected request parameter in
execution_strategy by adding a NumPy-style Parameters section for request:
pytest.FixtureRequest, describing that it selects the asynchronous or
synchronous execution strategy returned by the fixture.
In `@tests/helpers/maturin.py`:
- Around line 219-223: Shorten the docstring for the private helper
_locate_dist_info_wheel by replacing the structured Returns section with a
concise single-line summary, preserving only any non-obvious invariants required
by the repository conventions.
🪄 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: 936ef6f9-394a-4958-b042-563ba0a0c228
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (50)
benchmarks/benchmark_profile.pybenchmarks/ci_benchmark_ratchet_profile.pybenchmarks/comparison_analysis.pybenchmarks/ratchet_rust_performance.pybenchmarks/sinks.pybenchmarks/tee_profile_scenarios.pycuprum/_concurrent_config.pycuprum/_process_lifecycle.pycuprum/_streams.pycuprum/_streams_rs.pycuprum/adapters/logging_adapter.pycuprum/builders/args.pycuprum/builders/git.pycuprum/catalogue.pycuprum/concurrent.pycuprum/context/_policy.pycuprum/sh.pycuprum/unittests/__snapshots__/test_maturin_build.ambrcuprum/unittests/_pump_stream_dispatch_support.pycuprum/unittests/_tee_profile_concurrency_support.pycuprum/unittests/conftest.pycuprum/unittests/strategies.pycuprum/unittests/test_concurrency.pycuprum/unittests/test_context.pycuprum/unittests/test_env_context.pycuprum/unittests/test_fixture_generation.pycuprum/unittests/test_pipeline_process_lifecycle.pycuprum/unittests/test_pipeline_wait.pycuprum/unittests/test_pump_stream_dispatch_fd_blocking.pycuprum/unittests/test_pump_stream_dispatch_selection.pycuprum/unittests/test_safe_cmd_context.pycuprum/unittests/test_safe_cmd_run.pycuprum/unittests/test_safe_cmd_stdin.pycuprum/unittests/test_stream_parity.pycuprum/unittests/test_stream_pump_runtime_behaviour.pycuprum/unittests/test_tee_profile_worker_selector_metrics.pycuprum/unittests/test_validation.pydocs/developers-guide.mdpyproject.tomlscripts/tests/test_typos_rollout.pytests/behaviour/_context_hooks_support.pytests/behaviour/_execution_runtime_support.pytests/behaviour/test_backend_dispatcher_behaviour.pytests/behaviour/test_benchmark_ci_ratchet_behaviour.pytests/behaviour/test_benchmark_comparison_report_behaviour.pytests/behaviour/test_pipeline_execution.pytests/behaviour/test_stream_fidelity.pytests/helpers/maturin.pytests/helpers/maturin_pins.pytests/helpers/stream_pipes.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/shared-actions(auto-detected)leynos/pylint-pypy-shim(auto-detected)leynos/whitaker(auto-detected)
| @pytest.mark.parametrize( | ||
| "case", | ||
| [ | ||
| pytest.param( | ||
| { | ||
| "backend_env": "python", | ||
| "rust_available": None, | ||
| "force_fd_extraction_failure": False, | ||
| "expected_rust_fd_attempts": 0, | ||
| }, | ||
| id="forced-python", | ||
| ), | ||
| pytest.param( | ||
| { | ||
| "backend_env": "rust", | ||
| "rust_available": True, | ||
| "force_fd_extraction_failure": True, | ||
| "expected_rust_fd_attempts": 1, | ||
| }, | ||
| id="rust-fd-extraction-fails", | ||
| ), | ||
| ], | ||
| ) | ||
| def test_dispatch_falls_back_to_python( | ||
| self, | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| case: dict[str, str | bool | int | None], | ||
| ) -> None: | ||
| """Python pump is used when forced or when Rust FD extraction fails. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| monkeypatch : pytest.MonkeyPatch | ||
| Fixture used to override environment variables. | ||
| case : dict[str, str | bool | None | int] | ||
| Parameterized backend mode and expected call counts. | ||
| """ | ||
| _ = self | ||
| backend_env = typ.cast("str", case["backend_env"]) | ||
| rust_available = typ.cast("bool | None", case["rust_available"]) | ||
| force_fd_extraction_failure = typ.cast( | ||
| "bool", case["force_fd_extraction_failure"] | ||
| ) | ||
| expected_rust_fd_attempts = typ.cast("int", case["expected_rust_fd_attempts"]) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Replace the loosely-typed parametrize case dict with a TypedDict.
case: dict[str, str | bool | int | None] forces five typ.cast() calls to recover each field's real type. Define a TypedDict for the four fields instead, so Pyright can check field types directly without casts.
♻️ Proposed refactor
+class _FallbackCase(typ.TypedDict):
+ backend_env: str
+ rust_available: bool | None
+ force_fd_extraction_failure: bool
+ expected_rust_fd_attempts: int
+
+
`@pytest.mark.parametrize`(
"case",
[
pytest.param(
{
"backend_env": "python",
"rust_available": None,
"force_fd_extraction_failure": False,
"expected_rust_fd_attempts": 0,
},
id="forced-python",
),
...
],
)
def test_dispatch_falls_back_to_python(
self,
monkeypatch: pytest.MonkeyPatch,
- case: dict[str, str | bool | int | None],
+ case: _FallbackCase,
) -> None:
...
- backend_env = typ.cast("str", case["backend_env"])
- rust_available = typ.cast("bool | None", case["rust_available"])
- force_fd_extraction_failure = typ.cast(
- "bool", case["force_fd_extraction_failure"]
- )
- expected_rust_fd_attempts = typ.cast("int", case["expected_rust_fd_attempts"])
+ backend_env = case["backend_env"]
+ rust_available = case["rust_available"]
+ force_fd_extraction_failure = case["force_fd_extraction_failure"]
+ expected_rust_fd_attempts = case["expected_rust_fd_attempts"]As per coding guidelines, "Use TypedDict or @dataclass(slots=True) for structured data instead of plain dictionaries or classes."
📝 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.
| @pytest.mark.parametrize( | |
| "case", | |
| [ | |
| pytest.param( | |
| { | |
| "backend_env": "python", | |
| "rust_available": None, | |
| "force_fd_extraction_failure": False, | |
| "expected_rust_fd_attempts": 0, | |
| }, | |
| id="forced-python", | |
| ), | |
| pytest.param( | |
| { | |
| "backend_env": "rust", | |
| "rust_available": True, | |
| "force_fd_extraction_failure": True, | |
| "expected_rust_fd_attempts": 1, | |
| }, | |
| id="rust-fd-extraction-fails", | |
| ), | |
| ], | |
| ) | |
| def test_dispatch_falls_back_to_python( | |
| self, | |
| monkeypatch: pytest.MonkeyPatch, | |
| case: dict[str, str | bool | int | None], | |
| ) -> None: | |
| """Python pump is used when forced or when Rust FD extraction fails. | |
| Parameters | |
| ---------- | |
| monkeypatch : pytest.MonkeyPatch | |
| Fixture used to override environment variables. | |
| case : dict[str, str | bool | None | int] | |
| Parameterized backend mode and expected call counts. | |
| """ | |
| _ = self | |
| backend_env = typ.cast("str", case["backend_env"]) | |
| rust_available = typ.cast("bool | None", case["rust_available"]) | |
| force_fd_extraction_failure = typ.cast( | |
| "bool", case["force_fd_extraction_failure"] | |
| ) | |
| expected_rust_fd_attempts = typ.cast("int", case["expected_rust_fd_attempts"]) | |
| class _FallbackCase(typ.TypedDict): | |
| backend_env: str | |
| rust_available: bool | None | |
| force_fd_extraction_failure: bool | |
| expected_rust_fd_attempts: int | |
| `@pytest.mark.parametrize`( | |
| "case", | |
| [ | |
| pytest.param( | |
| { | |
| "backend_env": "python", | |
| "rust_available": None, | |
| "force_fd_extraction_failure": False, | |
| "expected_rust_fd_attempts": 0, | |
| }, | |
| id="forced-python", | |
| ), | |
| pytest.param( | |
| { | |
| "backend_env": "rust", | |
| "rust_available": True, | |
| "force_fd_extraction_failure": True, | |
| "expected_rust_fd_attempts": 1, | |
| }, | |
| id="rust-fd-extraction-fails", | |
| ), | |
| ], | |
| ) | |
| def test_dispatch_falls_back_to_python( | |
| self, | |
| monkeypatch: pytest.MonkeyPatch, | |
| case: _FallbackCase, | |
| ) -> None: | |
| """Python pump is used when forced or when Rust FD extraction fails. | |
| Parameters | |
| ---------- | |
| monkeypatch : pytest.MonkeyPatch | |
| Fixture used to override environment variables. | |
| case : _FallbackCase | |
| Parameterized backend mode and expected call counts. | |
| """ | |
| _ = self | |
| backend_env = case["backend_env"] | |
| rust_available = case["rust_available"] | |
| force_fd_extraction_failure = case["force_fd_extraction_failure"] | |
| expected_rust_fd_attempts = case["expected_rust_fd_attempts"] |
🤖 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 `@cuprum/unittests/test_pump_stream_dispatch_selection.py` around lines 29 -
72, Define a TypedDict for the parametrized case fields backend_env,
rust_available, force_fd_extraction_failure, and expected_rust_fd_attempts, then
annotate the pytest parameter and test_dispatch_falls_back_to_python case
argument with it. Remove the typ.cast calls and access each field directly so
Pyright validates the field types.
Source: Coding guidelines
Tests:
- test_concurrency: replace both type-ignore suppressions with
typ.cast("int", ...) so the invalid runtime-value tests keep their
coverage without silencing Pyright; add a diagnostic message to the
omitted-index backfill assertion.
- test_pump_stream_dispatch_selection: add a _DispatchCase TypedDict for
the parametrized fields, construct each case through it, and drop the
four typ.cast calls so field types are checked.
Docstrings:
- benchmarks: complete the public exception contracts —
validate_matching_profiles (missing/malformed profile metadata),
load_plan_payload (file, JSON, structural failures),
select_ci_ratchet_scenarios (strict-zip and field validation), main
(subprocess.CalledProcessError plus propagated loader/selection
failures), and compare_candidate_backend_results (malformed payload and
incomplete Python/Rust groups). Propagated exceptions carry scoped
DOC502 suppressions.
- _streams_rs: reorder rust_consume_stream so Notes follows Raises.
- Collapse redundant Returns sections on private/nested helpers: sh.py
builder, test_safe_cmd_run orchestrate, both _prepare_fixture_bundle
helpers, and maturin _locate_dist_info_wheel.
- conftest redact: describe dictionaries (not mappings) and clarify that
only dict/list containers are copied recursively.
- test_safe_cmd_stdin execution_strategy: document the injected request
parameter.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Gates Failed
Enforce critical code health rules
(1 file with Low Cohesion)
Our agent can fix these. Install it.
Gates Passed
4 Quality Gates Passed
Reason for failure
| Enforce critical code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| test_concurrency.py | 1 critical rule | 10.00 → 8.03 | 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.
| ConcurrentConfig(concurrency=-1) | ||
|
|
||
|
|
||
| def test_concurrency_bool_raises_type_error() -> None: |
There was a problem hiding this comment.
❌ New issue: Low Cohesion
This module has at least 34 different responsibilities amongst its 39 functions, threshold = 4
Summary
Enables Ruff
previewmode and theDOCrule group, and pins theD(pydocstyle) group to the numpy convention inpyproject.toml, turning the docstring signature/return/yield/raises contract into an enforced commit gate.Changes
pyproject.toml—preview = true(already set), addDOCto theselectlist, and keepconvention = "numpy"for pydocstyle.Returns,Raises, andYieldssections across ~140 Python files (library, benchmarks, tests) to satisfy the newly enforced rules, and removed extraneous return/exception entries where the docs did not match the code (807 findings resolved).C0302), so each was split along a coherent seam into a new sibling module, with the original public surface preserved via re-export shims:cuprum/_concurrent_config.py,cuprum/_pipeline_collect.py,cuprum/_pipeline_stream_results.py,cuprum/_streams_pump.py,cuprum/adapters/_tracing_protocols.pybenchmarks/ratchet_ratio_extraction.py,tests/helpers/maturin_pins.pytests/behaviour/_execution_runtime_support.py,tests/behaviour/_context_hooks_support.pycuprum/modules.Behaviour
No runtime behaviour changes — only docstrings, config, and pure code moves.
Verification
All commit gates pass:
make check-fmt,make lint(Ruff clean, interrogate 100%, pylint 10.00/10 with noC0302),make typecheck, andmake test(725+ Python tests and 33 Rust tests passing).References
🤖 Generated with Claude Code