Split cuprum/context.py into a context/ package (#116)

[leynos]

Summary

This branch splits the 763-line cuprum/context.py into a cuprum/context/ package with every module under 400 lines, preserving the public API surface unchanged.

Closes #116.

The module mixed four concerns with different audiences and change cadences. The split follows the seams named in the issue:

• cuprum/context/env_overlay.py — pure overlay merging (merge_env_overlays, resolve_env, _coerce_env_overlay); no ContextVar dependency.
• cuprum/context/core.py — CuprumContext, ScopeConfig, ForbiddenProgramError, timeout validation, hook type aliases.
• cuprum/context/state.py — the ContextVar machinery.
• cuprum/context/registration.py — scoped, the _TokenRegistration base, the handles, and the factories.
• cuprum/context/init.py — re-exports the existing public __all__ unchanged.

Review walkthrough

• The code moves verbatim apart from import plumbing; review the new module boundaries and __init__ surface first, then skim the per-module imports.
• docs/developers-guide.md documents the package layout and where new context features belong.

Validation

• make check-fmt: pass
• make lint: pass (no module over 400 lines; largest is registration.py at 372)
• make typecheck: pass
• make test: pass (727 passed, 50 skipped; the public-API, context, env-overlay, and stateful registration suites pass unchanged; Rust suite 4 passed)
• make markdownlint: pass
• coderabbit review --agent: invoked after the final push; the CLI began analysis but returned no terminal findings report

Notes

This branch is stacked on #156 (#113) per the issue's coordination note, so registration.py lands already de-duplicated; it should be rebased once #156 merges. The wheel-build snapshot reflects the new file list.

References

• Lody session

[sourcery-ai]

Sorry @leynos, you have reached your weekly rate limit of 2500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Split the former cuprum/context.py module into a package containing context types, environment helpers, ContextVar state, and registration handles. Re-export the public API, update packaging and documentation, revise benchmark ratcheting, preserve Rust error conversion details, and centralize concurrent pipe-test handling.

Changes

Execution context package

Layer / File(s)	Summary
Context domain and environment overlays cuprum/context/core.py, cuprum/context/env_overlay.py, cuprum/unittests/test_context.py	Define immutable context types, allowlist enforcement, hook and timeout handling, environment overlay merging, and validation tests.
Context state and registration lifecycle cuprum/context/state.py, cuprum/context/registration.py, cuprum/unittests/test_token_registration_stateful.py	Provide ContextVar access and scoped registration handles with token restoration and nested environment-detach coverage.
Package exports and distribution documentation cuprum/context/__init__.py, cuprum/unittests/__snapshots__/test_maturin_build.ambr, docs/adr-006-context-package-split.md, docs/developers-guide.md	Expose the split implementation, update wheel contents, and document module ownership and registration protocols.

Benchmark ratchet measurement

Layer / File(s)	Summary
Deterministic benchmark profile benchmarks/*, cuprum/unittests/test_benchmark_ci_ratchet.py, cuprum/unittests/test_ci_benchmark_ratchet_profile.py	Configure ten benchmark runs, sort selected scenarios deterministically, validate scenario metadata, reject the previous profile version, and update tests.
Benchmark investigation record docs/debugging/*, docs/cuprum-design.md, docs/developers-guide.md, docs/users-guide.md	Record investigation hypotheses and evidence, and describe the revised benchmark measurement and compatibility procedures.

Rust pump I/O and error handling

Layer / File(s)	Summary
Rust OS error conversion rust/cuprum-rust/src/errors.rs, docs/developers-guide.md	Document preservation of specific Python I/O exception types during Rust error conversion.
Concurrent pump test plumbing tests/helpers/stream_pipes.py, cuprum/unittests/test_rust_splice.py, cuprum/unittests/test_rust_streams.py, tests/behaviour/test_rust_streams_behaviour.py	Run pipe writers and readers concurrently around the native pump and reuse shared helpers across unit and behaviour tests.

Possibly related PRs

• leynos/cuprum#136: Both changes modify how ExecHook is exposed through cuprum.context.
• leynos/cuprum#155: Uses the context package’s environment and observation APIs.
• leynos/cuprum#156: Shares the token-based registration handle protocol.

Suggested labels: Issue

Poem

Four modules clear the crowded room,
Context and overlays bloom.
Tokens restore each scoped state,
Ten-run benchmarks calibrate.
Pipes flow; errors keep their code.

🚥 Pre-merge checks | ✅ 18 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	Several benchmark, stream-helper, Rust, and docs edits sit outside the context-package split.	Move unrelated benchmark, Rust, and helper changes into separate PRs, or justify them in #116.
User-Facing Documentation	⚠️ Warning	users-guide.md documents the benchmark ratchet skip, but omits that incompatible baselines also write a skip report, which the code does.	Update the CI build commands section to say incompatible baselines produce a skip report, not just a skipped comparison.

✅ Passed checks (18 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the context package split and includes the linked issue reference (#116).
Description check	✅ Passed	The description tracks the same context-package split and stays on-topic.
Linked Issues check	✅ Passed	The package split, re-exports, module boundaries, and under-400-line limits match issue #116.
Docstring Coverage	✅ Passed	Docstring coverage is 99.04% which is sufficient. The required threshold is 80.00%.
Testing (Overall)	✅ Passed	Substantive coverage exists: property tests, a Hypothesis state machine, real subprocess env tests, and observe/benchmark checks would fail on plausible broken implementations.
Developer Documentation	✅ Passed	docs/developers-guide.md documents the new context package layout, and ADR-006 records the split decision; no execplan or roadmap item applies.
Module-Level Documentation	✅ Passed	All changed Python modules carry top-level docstrings, and the new context package modules explain their roles and relationships to sibling modules.
Testing (Unit And Behavioural)	✅ Passed	PASS: Unit tests cover invariants, edge cases, and errors; BDD tests hit public context and subprocess env boundaries, not private helpers.
Testing (Property / Proof)	✅ Passed	Hypothesis suites and a state machine already cover allowlist, timeout, env-overlay and token-restoration invariants.
Testing (Compile-Time / Ui)	✅ Passed	PASS: Rust compile-time coverage already uses trybuild UI tests, and the wheel-build snapshot is focused, redacted, and paired with semantic assertions.
Unit Architecture	✅ Passed	PASS: Query helpers stay read-only; mutation and side-effects are isolated to registration handles and subprocess-boundary helpers.
Domain Architecture	✅ Passed	PASS: domain-facing context logic now sits in core/state/registration, while the only os.environ coupling is isolated in env_overlay.py; no adapter leakage into core was found.
Observability	✅ Passed	PASS: The commit only edits docs/developers-guide.md; no runtime code, logs, metrics, traces, or alerts changed, so observability requirements are not applicable.
Security And Privacy	✅ Passed	Treat this as safe: it is a package split only, with no new secrets, credentials, auth gaps, or unsafe sinks; the argv note warns about existing logging behaviour.
Performance And Resource Use	✅ Passed	PASS: New loops are linear and bounded; env resolution copies os.environ only when needed, and worker threads are joined with a 10s cap.
Concurrency And State	✅ Passed	Accept it: ContextVar-backed state is immutable, thread/task isolation and token restoration are tested, and stream worker threads have bounded joins/cancellation.
Architectural Complexity And Maintainability	✅ Passed	PASS: The split isolates four real seams into sub-400-line modules, keeps init a thin façade, and uses _TokenRegistration only to deduplicate shared token-restoration logic.
Rust Compiler Lint Integrity	✅ Passed	PASS: The Rust diff is comment-only; scans found no broad lint suppressions, touch helpers, or suspicious .clone() calls in the Rust sources.

📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #116

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-116-split-context-package

---

_{Comment @coderabbitai help to get the list of available commands.}

[leynos]

CodeScene suppression request — Code Duplication in cuprum/context/registration.py

CodeScene flags: "The module contains 2 functions with similar structure: after, before" (advisory rule, new-file health 9.39).

Suggested suppression message for the CodeScene UI:

File: cuprum/context/registration.py
Symbols: before, after (and the sibling factory observe)
Reason: The similarity is superficial. These are one-line public factory functions whose bodies differ only in the hook-kind literal, but they exist precisely to give each hook kind a distinct, nominally typed entry point: before(hook: BeforeHook), after(hook: AfterHook), and observe(hook: ExecHook) accept different callable signatures and carry user-facing docstrings with kind-specific examples. Folding them into one parameterised factory would erase that typed API surface (callers would lose static checking of the hook signature against the registration kind). The behaviour they share — ContextVar token capture, idempotent detach(), and the context-manager protocol — is already centralised in the _TokenRegistration base introduced by this PR stack, so no logic is duplicated; only the thin, deliberately parallel API veneer remains.

If preferred, I can instead merge the three factories behind typing.overload signatures, but that trades three honest one-liners for an overload stack plus a runtime dispatcher, which seems strictly worse for legibility.

[coderabbitai]

Actionable comments posted: 5

🤖 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/context/__init__.py`:
- Around line 75-101: Remove the six underscore-prefixed helper names from
__all__ in cuprum.context.__init__, while leaving their explicit module bindings
and implementations unchanged so direct imports and existing tests continue to
work.

In `@cuprum/context/registration.py`:
- Around line 119-126: Update Registration.detach so _detached is set only after
_reset_context successfully restores the captured token; keep the token intact
when restoration raises ValueError, allowing a retry in the originating context.
- Around line 191-207: Update the hook type branching in __init__ to explicitly
handle "before", "after", and "observe"; raise ValueError for any other value
instead of falling through to with_observe_hook. Preserve the existing hook
casting and context installation for the three supported discriminators.

In `@cuprum/unittests/test_token_registration_stateful.py`:
- Around line 109-113: Update
cuprum/unittests/test_token_registration_stateful.py at lines 109-113 to assert
current_context() is self._baseline after teardown drains _stack, preserving the
existing LIFO detach behavior. At lines 135-154, capture the caller context
before scoped(...) and assert current_context() is that context after scope
exit, while retaining the existing out-of-order detach assertions.
- Line 101: Update all four bare assertions in
cuprum/unittests/test_token_registration_stateful.py at lines 101, 107, 147, and
154 with the specified failure messages, covering first-detach restoration,
baseline restoration, inner-overlay discard, and captured-overlay restoration.
Preserve the existing assertion conditions and use assert-with-message syntax
throughout.

🪄 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: fb8ca6a2-ced2-43c1-8c68-407095bf325e

📥 Commits

Reviewing files that changed from the base of the PR and between 6238fbd and 73299d1.

📒 Files selected for processing (10)

• cuprum/context.py
• cuprum/context/__init__.py
• cuprum/context/core.py
• cuprum/context/env_overlay.py
• cuprum/context/registration.py
• cuprum/context/state.py
• cuprum/unittests/__snapshots__/test_maturin_build.ambr
• cuprum/unittests/test_token_registration_stateful.py
• docs/developers-guide.md
• typos.toml

🔗 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)

💤 Files with no reviewable changes (1)

• cuprum/context.py

[pandalump]

@coderabbitai resume

[coderabbitai]

✅ Action performed

Reviews resumed.

[coderabbitai]

Actionable comments posted: 3

♻️ Duplicate comments (3)

cuprum/context/__init__.py (1)

75-101: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Strip the underscore-prefixed helpers out of __all__.

__all__ still lists _merge_after_hooks, _merge_before_hooks, _merge_observe_hooks, _narrow_allowlist, _resolve_narrowed_timeout, and _validate_timeout. The former cuprum.context.__all__ excluded these; keep the explicit module-level bindings (so cuprum.context._validate_timeout etc. still resolve for existing tests) but stop advertising them through the public surface / wildcard imports.

🐛 Proposed fix

 __all__ = [
     "AfterHook",
     "AllowRegistration",
     "BeforeHook",
     "CuprumContext",
     "EnvRegistration",
     "ExecHook",
     "ForbiddenProgramError",
     "HookRegistration",
     "ScopeConfig",
-    "_merge_after_hooks",
-    "_merge_before_hooks",
-    "_merge_observe_hooks",
-    "_narrow_allowlist",
-    "_resolve_narrowed_timeout",
-    "_validate_timeout",
     "after",

🤖 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/context/__init__.py` around lines 75 - 101, Remove the
underscore-prefixed helper names from the __all__ list in the module while
leaving their module-level definitions and bindings unchanged, so direct access
such as cuprum.context._validate_timeout continues to work without exposing them
through wildcard imports.

cuprum/unittests/test_token_registration_stateful.py (2)

101-101: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Attach failure messages to every remaining bare assertion.

Lines 101, 107, 147, and 154 are still bare assert statements. Wire in the failure messages so a violated restoration invariant is identifiable from the test output alone.

Proposed assertion messages

-        assert after_first is prior
+        assert after_first is prior, "first detach must restore the prior context"

-            assert current_context() is self._baseline
+            assert current_context() is self._baseline, (
+                "an empty stack must restore the baseline context"
+            )

-        assert "CUPRUM_TEST_INNER" not in overlay
+        assert "CUPRUM_TEST_INNER" not in overlay, (
+            "outer detach must discard the inner overlay"
+        )

-        assert leaked.get("CUPRUM_TEST_OUTER") == "outer"
+        assert leaked.get("CUPRUM_TEST_OUTER") == "outer", (
+            "inner detach must restore its captured outer overlay"
+        )

As per path instructions, "Use assert …, "message" over bare asserts."

Also applies to: 147-147, 154-154, 107-107

🤖 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_token_registration_stateful.py` at line 101, Update the
bare assertions in the stateful token registration tests, including those around
after_first and the assertions at the other referenced locations, to use the
assert condition, message form. Provide each assertion with a clear message
identifying the violated restoration invariant.

Source: Path instructions

---

109-113: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert restoration after automatic cleanup and scope exit.

teardown() drains _stack but never asserts the state-machine baseline is actually restored afterwards, and test_out_of_order_detach_restores_outer_snapshot never checks that leaving the scoped(...) block restores the caller's own context. Either gap lets a final-step cleanup regression leak state silently.

Proposed test additions

 def teardown(self) -> None:
     """Detach any remaining handles in LIFO order."""
     while self._stack:
         handle, _prior = self._stack.pop()
         handle.detach()
+    assert current_context() is self._baseline, (
+        "teardown must restore the state machine baseline"
+    )

 def test_out_of_order_detach_restores_outer_snapshot() -> None:
     """..."""
+    caller_context = current_context()
     with scoped(ScopeConfig()):
         ...
         leaked = current_context().env_overlay or {}
         assert leaked.get("CUPRUM_TEST_OUTER") == "outer", (
             "inner detach must restore its captured outer overlay"
         )
+    assert current_context() is caller_context, (
+        "scope exit must restore the caller context"
+    )

As per coding guidelines, "New functionality and behavioral changes require substantive, non-vacuous tests" covering edge cases and functional boundaries.

Also applies to: 135-154

🤖 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_token_registration_stateful.py` around lines 109 - 113,
Update teardown() to assert that automatic cleanup restores the state-machine
baseline after draining _stack, using the existing baseline/state assertion
mechanism. Extend test_out_of_order_detach_restores_outer_snapshot to assert
that exiting the scoped(...) block restores the caller’s original context,
covering both automatic cleanup and scope-exit restoration.

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/context/core.py`:
- Line 14: Update the timeout validation logic in the affected context
configuration flow to reject all non-finite values, including NaN and positive
or negative infinity, before the existing negative-value check proceeds. Use the
existing timeout validation symbols and raise ValueError consistently for these
invalid inputs.
- Around line 31-32: Define or reuse a package-level domain exception base, then
update ForbiddenProgramError to inherit from both that base and PermissionError.
Extend its initialization to accept and store the denied program and
restricted_state as attributes while preserving the existing descriptive
message.

In `@cuprum/unittests/test_token_registration_stateful.py`:
- Around line 103-107: Update stack tracking in the stateful test so each
registration stores both its prior context and installed context in self._stack.
Expand stack_depth_matches_context_nesting to assert current_context() matches
the baseline when empty and the top entry’s installed context when nested, and
adjust every self._stack.pop() unpacking site to handle the two stored values.

---

Duplicate comments:
In `@cuprum/context/__init__.py`:
- Around line 75-101: Remove the underscore-prefixed helper names from the
__all__ list in the module while leaving their module-level definitions and
bindings unchanged, so direct access such as cuprum.context._validate_timeout
continues to work without exposing them through wildcard imports.

In `@cuprum/unittests/test_token_registration_stateful.py`:
- Line 101: Update the bare assertions in the stateful token registration tests,
including those around after_first and the assertions at the other referenced
locations, to use the assert condition, message form. Provide each assertion
with a clear message identifying the violated restoration invariant.
- Around line 109-113: Update teardown() to assert that automatic cleanup
restores the state-machine baseline after draining _stack, using the existing
baseline/state assertion mechanism. Extend
test_out_of_order_detach_restores_outer_snapshot to assert that exiting the
scoped(...) block restores the caller’s original context, covering both
automatic cleanup and scope-exit restoration.

🪄 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: 6f447008-5fd5-4870-b930-c1fba8f36580

📥 Commits

Reviewing files that changed from the base of the PR and between 6238fbd and 73299d1.

📒 Files selected for processing (10)

• cuprum/context.py
• cuprum/context/__init__.py
• cuprum/context/core.py
• cuprum/context/env_overlay.py
• cuprum/context/registration.py
• cuprum/context/state.py
• cuprum/unittests/__snapshots__/test_maturin_build.ambr
• cuprum/unittests/test_token_registration_stateful.py
• docs/developers-guide.md
• typos.toml

🔗 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)

💤 Files with no reviewable changes (1)

• cuprum/context.py

[leynos]

@coderabbitai Please suggest a fix for the following concern and provide an AI coding agent prompt for the fix:

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Testing (Overall)	❌ Error	The new stateful suite is weak: it only checks empty-stack restoration, so nested env registrations can overwrite outer overlays without failing.	Add a nested env-registration test with disjoint keys and strengthen the stateful invariant to check the top stack context at every step.

[pandalump]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 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 `@docs/users-guide.md`:
- Around line 1491-1493: Update the incompatible older benchmark-profile
baseline documentation in users-guide.md to state that, in addition to skipping
comparison, the system writes a skip report. Keep the explanation about
differing sampling protocols and worker timings unchanged.

🪄 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: fdd87ab1-343c-467d-8f1f-025e2837d062

📥 Commits

Reviewing files that changed from the base of the PR and between dacb76d and a28c9db.

📒 Files selected for processing (24)

• benchmarks/_validation.py
• benchmarks/benchmark_profile.py
• benchmarks/ci_benchmark_ratchet_profile.py
• cuprum/context.py
• cuprum/context/__init__.py
• cuprum/context/core.py
• cuprum/context/env_overlay.py
• cuprum/context/registration.py
• cuprum/context/state.py
• cuprum/unittests/__snapshots__/test_maturin_build.ambr
• cuprum/unittests/test_benchmark_ci_ratchet.py
• cuprum/unittests/test_ci_benchmark_ratchet_profile.py
• cuprum/unittests/test_context.py
• cuprum/unittests/test_rust_splice.py
• cuprum/unittests/test_rust_streams.py
• cuprum/unittests/test_token_registration_stateful.py
• docs/adr-006-context-package-split.md
• docs/cuprum-design.md
• docs/debugging/debugging-plan-2026-07-19T17-32-12Z.md
• docs/developers-guide.md
• docs/users-guide.md
• rust/cuprum-rust/src/errors.rs
• tests/behaviour/test_rust_streams_behaviour.py
• tests/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)

💤 Files with no reviewable changes (1)

• cuprum/context.py

[codescene-access]

No quality gates enabled for this code.