Skip to content

📝 CodeRabbit Chat: Implement requested code changes - #146

Open
coderabbitai[bot] wants to merge 3 commits into
mainfrom
coderabbitai/chat/87657ee
Open

📝 CodeRabbit Chat: Implement requested code changes#146
coderabbitai[bot] wants to merge 3 commits into
mainfrom
coderabbitai/chat/87657ee

Conversation

@coderabbitai

@coderabbitai coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code changes was requested by @leynos.

The following files were modified:

  • cuprum/unittests/__snapshots__/test_maturin_build.ambr
  • cuprum/unittests/__snapshots__/test_tee_profile_worker_concurrency.ambr
  • cuprum/unittests/__snapshots__/test_tee_profile_worker_reentrancy.ambr
  • cuprum/unittests/conftest.py
  • cuprum/unittests/test_tee_profile_worker_concurrency.py
  • cuprum/unittests/test_tee_profile_worker_env_preservation.py
  • cuprum/unittests/test_tee_profile_worker_reentrancy.py

Summary by Sourcery

Extract backend environment-preservation and selector reentrancy tests for tee profile workers into dedicated modules and share common concurrency utilities across tests.

Tests:

  • Add focused tests verifying concurrent tee profile workers preserve backend environment state and lock-serialised observations.
  • Add dedicated tests covering backend selector reentrancy, including nested selector failures and structured warning logs.
  • Refactor concurrency tests to use shared backend pair helpers and thread-join utilities from conftest.

leynos and others added 3 commits June 10, 2026 23:53
`make lint` fails on `main` because the interrogate gate added in #131
requires 100% docstring coverage, but five members of
`test_tee_profile_worker_concurrency.py` (`_RLockLike.acquire`,
`_RLockLike.release`, `_SignallingRLock.release`, `__enter__`, and
`__exit__`) lack docstrings. Add them.

`make test` also flakes on `test_crosshair_contracts[split_no_text_loss]`:
CrossHair needs roughly 15 seconds to confirm the contract on a loaded
6-core host, so the 10-second `per_condition_timeout` intermittently
yields `CANNOT_CONFIRM`. Raise the budget to 60 seconds; confirmation
exits early on success, so the common-case runtime is unchanged.
The PIPE-versus-DEVNULL stdio selection for spawning pipeline stages was
duplicated: `_spawn_pipeline_processes` re-derived the flags inline while
the purpose-built `_get_stage_stream_fds` helper sat dead behind a
self-referential re-export in `cuprum/_pipeline_streams.py`.

- Route `_spawn_pipeline_processes` through `_get_stage_stream_fds`,
  making the helper the single source of truth for the stage stdio
  policy, and document the policy on its docstring.
- Extract `_cwd_arg(cwd)` into `cuprum/_subprocess_context.py` and use it
  from both the single-command (`_spawn_subprocess`) and pipeline spawn
  sites, removing the duplicated
  `str(...) if ... is not None else None` idiom.
- Remove the dead `_get_stage_stream_fds` / `_StageStreamConfig`
  re-exports now that a real caller imports from
  `cuprum._pipeline_stage_streams` directly.
- Cover the full finite input domain (stage position x capture/echo) in
  `cuprum/unittests/test_stage_stream_fds.py`, including agreement with
  the single-command stdout/stderr policy on overlapping cases and
  `_cwd_arg` conversion cases.
- Document the canonical helpers and their re-use policy in
  `docs/developers-guide.md`.

The wheel-build snapshot is refreshed because it records the package
file list, which now includes the new test module.
@coderabbitai
coderabbitai Bot requested a review from leynos June 11, 2026 10:33
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Author

Important

Review skipped

This PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: cf96dcf8-01e7-4b32-829f-ca0d9d71819c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands and usage tips.

@sourcery-ai

sourcery-ai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Refactors tee_profile worker concurrency tests by extracting shared threading utilities into conftest, splitting environment-preservation and reentrancy scenarios into dedicated test modules, and updating snapshots accordingly.

File-Level Changes

Change Details Files
Refactor concurrency tests to reuse shared thread-joining utilities from conftest.
  • Removed local timeout constants and thread-join helper from the concurrency test module.
  • Imported shared _EVENT_WAIT_TIMEOUT_SECONDS and join_and_assert_finished from conftest.
  • Simplified backend pair helper utilities to rely on a shared alternate-backend resolver.
cuprum/unittests/test_tee_profile_worker_concurrency.py
cuprum/unittests/conftest.py
Introduce shared threading utilities in conftest for test modules.
  • Defined global _EVENT_WAIT_TIMEOUT_SECONDS and _THREAD_JOIN_TIMEOUT_SECONDS constants for use across tests.
  • Added join_and_assert_finished helper to join multiple threads and assert they terminate within a common timeout.
cuprum/unittests/conftest.py
Extract environment-preservation concurrency tests into a dedicated module with instrumented selectors.
  • Introduced coordinated and checkpoint backend selector stand-ins to instrument _EnvBackendSelector behavior.
  • Implemented _BackendEnvironmentRace harness and selector-context helpers to exercise concurrent worker behavior.
  • Added tests ensuring concurrent workers preserve CUPRUM_STREAM_BACKEND and that selector interleaving is properly serialised via lock contention.
  • Updated/added corresponding snapshot expectations.
cuprum/unittests/test_tee_profile_worker_env_preservation.py
cuprum/unittests/__snapshots__/test_tee_profile_worker_concurrency.ambr
cuprum/unittests/__snapshots__/test_maturin_build.ambr
Extract reentrancy and lock-primitive tests for _EnvBackendSelector into a dedicated module.
  • Added hypothesis-based strategy to generate available backend pairs based on runtime backend availability.
  • Implemented tests verifying backend lock reentrancy, nested selector rejection semantics, and recovery behavior.
  • Introduced logging-based test that snapshots structured reentrancy warning records to ensure stable log shape.
  • Added snapshot file for reentrancy warning expectations.
cuprum/unittests/test_tee_profile_worker_reentrancy.py
cuprum/unittests/__snapshots__/test_tee_profile_worker_reentrancy.ambr

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@lodyai
lodyai Bot force-pushed the issue-112-canonical-pipeline-stdio-policy branch 6 times, most recently from 20a7c64 to 5916b59 Compare June 16, 2026 00:19
@lodyai
lodyai Bot force-pushed the issue-112-canonical-pipeline-stdio-policy branch 2 times, most recently from 9a861b8 to e7fe4ac Compare July 2, 2026 21:35
@lodyai
lodyai Bot force-pushed the issue-112-canonical-pipeline-stdio-policy branch 4 times, most recently from d5ab9b1 to d7f0737 Compare July 14, 2026 22:51
Base automatically changed from issue-112-canonical-pipeline-stdio-policy to main July 15, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant