Add structured timeout and teardown observability (#284)

[leynos]

Summary

Closes #284.

Stacked on top of #223 (base: issue-221-…). Splitting out at maintainer
request — the observability and documentation work grew beyond the ASYNC lint
enablement that #221 asked for, so it now ships as its own reviewable change.
Merge #223 first; this PR's base will retarget to main automatically.

Timeout and teardown failures were previously invisible: a TimeoutExpired
told a caller that a command timed out, but nothing recorded how (an
elapsed deadline versus an immediate non-positive expiry), and a stream-drain
failure during teardown was swallowed entirely. This adds structured
diagnostics on both existing channels — the cuprum.timeout logger and the
ExecEvent observe stream — without introducing a parallel telemetry
framework.

Changes

Event contract

• ExecPhase gains two ancillary phases, timeout and teardown_error.
  They are ancillary in the sense that they do not participate in the
  plan → start → exit lifecycle: they may fire zero or one times and never
  replace a lifecycle phase.
• ExecEvent gains timeout_s: float | None and timeout_mode: str | None,
  populated on the timeout phase. timeout_mode distinguishes
  elapsed_deadline from non_positive_immediate.
• pid is now documented as populated for every phase except plan, rather
  than enumerating a list that had gone stale twice.

Emission

• _report_timeout_expiry and _report_teardown_drain_failure each pair one
  log record with one observe event from a single set of facts, so the two
  channels cannot drift. test_timeout_telemetry_pairing pins that invariant
  as a property test over generated pids, timeouts, and modes.
• Emission is best-effort: _emit_timeout_log suppresses Exception, and
  _safe_emit suppresses Exception and CancelledError, so a broken
  consumer cannot change what a caller sees.

Exception precedence

• _run_subprocess_with_streams and _run_subprocess_without_streams now
  clean up on except BaseException, after the timeout-specific clause, so a
  non-TimeoutError escape no longer leaks the stdin writer or the stream
  consumers.
• Failing async observe hooks previously replaced the error that ended the
  run — a hook raising during cleanup could stand in for the TimeoutExpired
  or CancelledError a caller was waiting to catch. _execute_with_hooks now
  drains through _drain_tasks_during_cleanup, which aggregates the hook
  failure with the active error into a BaseExceptionGroup. Only async hooks
  were affected (synchronous ones are swallowed by _safe_emit), which is why
  the pre-existing test passed while the bug was live; both new tests are
  mutation-verified to fail against the pre-fix code.
• _drain_tasks_during_cleanup's message label is required rather than
  defaulted, so a non-pipeline caller cannot silently inherit the pipeline's
  finalization label.

Adapters

• Metrics counts both new phases and dispatches via match/case.
• Tracing records the ancillary phases as span events without ending the
  span, and carries timeout_s/timeout_mode so a consumer can tell the two
  expiry modes apart.
• Logging passes both through unchanged (it is phase-tolerant by design).

Refactoring

• _execute_subprocess's direct path is extracted to
  _run_subprocess_without_streams (CodeScene).

Documentation

• docs/users-guide.md: the teardown_error contract, the new metrics
  counters, the cuprum.timeout log fields, and the tracing span-event
  behaviour.
• docs/developers-guide.md: the emission helpers and the pairing invariant.
• ADR-007: an addendum narrowing the "no observable change" claim to the public
  API, TimeoutExpired and its payload, and timeout/exception precedence,
  naming the telemetry as additive.
• test_async_timeout_docs pins the documented contract against the code, so
  the docs cannot drift silently.

Verification

• make check-fmt, make lint, make typecheck, make test, markdownlint
  and nixie all pass.
• coderabbit review --agent reports zero findings against this branch's own
  base (the issue-221-… branch, not main).
• The tip of this branch is byte-identical to the pre-split branch tip, so the
  split moved commits without altering the result.
• The documentation tests are mutation-verified: 19 of the 27 new parameters in
  test_async_timeout_docs fail against the pre-edit users' guide.

Notes for the reviewer

• The stack order is forced, not a preference. This PR cannot be moved
  below Enable the ASYNC (flake8-async) ruff rule group (#221) #223 without re-implementing it. The timeout telemetry is emitted from
  inside _wait_for_exit_code_within_timeout, and that helper is created by
  the ASYNC refactor (c6e7427) — it does not exist on main, where
  _wait_for_exit_code still calls asyncio.wait_for(process.wait(), timeout)
  directly. Cherry-picking these commits onto main fails at Emit timeout
  observe events from the wait path with
  _wait_for_exit_code_within_timeout: deleted in ours, modified in theirs.
  Inverting the stack would mean rewriting the observability against the
  pre-refactor wait_for code and then re-deriving the refactor on top of it,
  so please review this as landing after Enable the ASYNC (flake8-async) ruff rule group (#221) #223 rather than alongside it.

• One commit message on this branch (Action review findings on cleanup, telemetry and lint scope) describes a pyproject.toml ASYNC scope
  narrowing that now lives in Enable the ASYNC (flake8-async) ruff rule group (#221) #223 instead; the change itself was not lost,
  only relocated to the parent PR where it belongs.

• cuprum/adapters/tracing_adapter.py sits at exactly 400 lines, pylint's
  enforced max-module-lines. There is no headroom: the next line added to
  that file breaks the build. Extracting its two Protocol classes (~88
  lines) into their own module would be the natural remedy, but that is out of
  scope here.

References

• Tracking issue: Track timeout observability contract delivered by #271 #284
• Parent PR: Enable the ASYNC (flake8-async) ruff rule group (#221) #223 (stack #272)
• Issue: Enable the ASYNC (flake8-async) Ruff rule group and resolve its findings #221
• Lody session: https://lody.ai/leynos/sessions/4501495f-9c83-4ef7-98a3-5d62dcfb38a2

🤖 Generated with Claude Code

[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

Summary

• Add structured timeout and teardown_error observability to cuprum.timeout and ExecEvent.
• Emit paired logs and observation events with timeout duration, mode, process ID, and error details.
• Keep telemetry best-effort so logging or observation failures do not mask primary errors.
• Clean up subprocess and hook tasks across cancellation, timeout, teardown, and unexpected failures.
• Aggregate teardown failures without replacing the primary exception.
• Extend metrics and tracing adapters for the new phases.
• Extract subprocess wait, teardown, and no-stream execution helpers.
• Preserve positional compatibility for ExecEvent.exec_id.
• Add pipeline timeout reporting.
• Update ADR-007 and related design documentation.
• Add comprehensive documentation, pairing, adapter, cleanup, pipeline, and exception-precedence tests.

Walkthrough

The change adds structured timeout and teardown-error reporting for subprocesses and pipelines. It adds timeout metadata to execution events, preserves primary exceptions during cleanup, extends metrics and tracing projections, and adds tests and documentation.

Changes

Timeout observability

Layer / File(s)	Summary
Telemetry contracts and reporting cuprum/events.py, cuprum/_pipeline_types.py, cuprum/_timeout_reporting.py	Adds timeout and teardown-error phases, timeout metadata, structured logging, and best-effort observation emission.
Subprocess and pipeline cleanup cuprum/_subprocess_wait.py, cuprum/_subprocess_execution.py, cuprum/_pipeline_internals.py, cuprum/_pipeline_results.py, cuprum/sh.py	Centralizes process waiting, drains consumers once, reports expiry, emits pipeline exit events, and aggregates cleanup failures.
Metrics and tracing projections cuprum/adapters/metrics_adapter.py, cuprum/adapters/tracing_adapter.py, cuprum/unittests/test_adapter_projection.py, cuprum/unittests/test_tracing_adapter.py	Adds timeout and teardown-error counters, span fields, ancillary tracing phases, and projection snapshots.
Runtime and contract validation cuprum/unittests/*	Tests timeout modes, pipeline reporting, cleanup aggregation, telemetry isolation, event compatibility, adapter projections, documentation, and packaging.
Behaviour documentation docs/adr-007-subprocess-execution-module-boundaries.md, docs/developers-guide.md, docs/users-guide.md, docs/cuprum-design.md, CHANGELOG.md	Documents timeout handling, telemetry fields, cleanup guarantees, metrics, logging, tracing, and the release entry.

Sequence Diagram(s)

sequenceDiagram
  participant Pipeline
  participant SubprocessWait
  participant TimeoutReporting
  participant MetricsHook
  participant TracingHook
  Pipeline->>SubprocessWait: apply deadline and wait for stages
  SubprocessWait->>TimeoutReporting: report expiry or drain failure
  TimeoutReporting->>MetricsHook: emit timeout or teardown_error phase
  TimeoutReporting->>TracingHook: emit ancillary span event
  SubprocessWait-->>Pipeline: preserve primary exception

Loading

Possibly related PRs

• leynos/cuprum#223 — Provides the subprocess timeout waiting paths extended by this change.
• leynos/cuprum#226 — Shares subprocess stream-draining and timeout-cleanup paths.
• leynos/cuprum#158 — Shares subprocess wait and cleanup functions extended by this change.

Suggested labels: Issue

Suggested reviewers: codescene-access

Poem

Report each deadline and drain,
Keep the primary error plain.
Events carry mode and time,
Hooks cannot change the crime.
Metrics count and spans remain.

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 warning, 2 inconclusive)

Check name	Status	Explanation	Resolution
Developer Documentation	⚠️ Warning	The guide documents timeout telemetry and wait behaviour, but its module-boundary list omits new _subprocess_wait and _pipeline_results APIs; design docs omit the latter.	Document ownership, exports, and call boundaries for both modules in docs/developers-guide.md and docs/cuprum-design.md; document the required cleanup message contract and correct stale wait-helper wording.
Unit Architecture	❓ Inconclusive	Assessment pending repository inspection.	Inspect the changed execution, telemetry, and cleanup units and verify their dependency boundaries.
Domain Architecture	❓ Inconclusive	The changed modules mix process transport with telemetry reporting; the architecture check requires confirming whether this library treats subprocess execution as domain logic or infrastructure.	Review the package layering and timeout reporting call sites.

✅ Passed checks (17 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title describes the structured timeout and teardown observability changes and includes the linked issue reference (#284).
Description check	✅ Passed	The description clearly explains the timeout and teardown observability changes, their error-handling guarantees, and the related implementation work.
Linked Issues check	✅ Passed	The changes emit distinct timeout signals, report teardown failures, and prevent telemetry failures from masking primary exceptions as required by [#284].
Out of Scope Changes check	✅ Passed	The refactoring, adapter updates, documentation, and tests directly support the linked observability and exception-precedence objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Testing (Overall)	✅ Passed	Accept the testing: real command and pipeline runs, deterministic timeout cases, property tests, adapter assertions, telemetry pairing, cleanup checks, and exception-precedence tests exercise the c...
User-Facing Documentation	✅ Passed	Pass the check: docs/users-guide.md clearly documents timeout modes, TimeoutExpired behaviour, teardown errors, logging, observe events, metrics, and tracing; contract tests enforce these sections.
Module-Level Documentation	✅ Passed	Keep the module-level documentation: all 27 changed Python modules have docstrings that state their purpose, with private-module relationships described where relevant.
Testing (Unit And Behavioural)	✅ Passed	Accept: tests cover immediate and elapsed timeouts, cancellation, drain failures, telemetry pairing, exception precedence, and real SafeCmd and Pipeline workflows through public APIs.
Testing (Property / Proof)	✅ Passed	Hypothesis tests generate PIDs, timeout floats, modes, and ordered error lists, then verify one log/event pair, matching fields, and preserved error order.
Testing (Compile-Time / Ui)	✅ Passed	Treat compile-time tests as not applicable: the branch changes no Rust or TypeScript files; focused syrupy snapshots cover each phase with redacted volatile fields, plus semantic timeout and teardo...
Observability	✅ Passed	Timeout and teardown boundaries emit structured logs with stable fields, bounded counters, and correlated tracing events; best-effort emission preserves primary failures and docs/tests cover both t...
Security And Privacy	✅ Passed	Keep the change: added logs contain only PID, timeout metadata, and exception class names; added-line scans found no credentials, and execution still uses allowlists and create_subprocess_exec.
Performance And Resource Use	✅ Passed	New work is linear in pipeline stages or the fixed two stream consumers; telemetry joins at most two error types, drains tasks once, and clears pending hook tasks. No retry or nested hot-path loop...
Concurrency And State	✅ Passed	Keep this passing: task ownership and event-loop state are documented, teardown is shielded, and tests cover cancellation interleavings, consumer draining, async hook failures, and concurrent stage...
Architectural Complexity And Maintainability	✅ Passed	Keep the change: _subprocess_wait serves both stream paths, _timeout_reporting serves command and pipeline paths, and ADR-007 documents the boundaries; no runtime dependency changed.
Rust Compiler Lint Integrity	✅ Passed	Treat this check as not applicable: the complete diff against main contains no Rust, Cargo, or Rust lint-suppression changes, and adds no Rust clone calls.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch observability-enhancements

---

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

[sourcery-ai]

Reviewer's Guide

Adds structured, best-effort observability for subprocess timeouts and teardown drain failures across logging, observe events, metrics, and tracing, refactors subprocess execution timeout handling to be caller-owned via asyncio.timeout, and hardens cleanup so telemetry and hook failures cannot mask primary errors, while updating documentation and tests to lock the contracts in place.

Sequence diagram for subprocess timeout observability pair

sequenceDiagram
    actor User
    participant SafeCmd_run
    participant _execute_subprocess
    participant _wait_for_exit_code_within_timeout
    participant _report_timeout_expiry
    participant _log_timeout_expiry
    participant _emit_timeout_event
    participant _StageObservation

    User->>SafeCmd_run: run(timeout)
    SafeCmd_run->>_execute_subprocess: _execute_subprocess(execution)
    _execute_subprocess->>_wait_for_exit_code_within_timeout: _wait_for_exit_code_within_timeout(process, execution)

    alt timeout <= 0 (non_positive_immediate)
        _wait_for_exit_code_within_timeout->>_report_timeout_expiry: _report_timeout_expiry(observation, pid, configured_timeout, "non_positive_immediate")
    else timeout > 0 (elapsed_deadline)
        _wait_for_exit_code_within_timeout-->>_wait_for_exit_code_within_timeout: asyncio.timeout(timeout) expires
        _wait_for_exit_code_within_timeout->>_report_timeout_expiry: _report_timeout_expiry(observation, pid, configured_timeout, "elapsed_deadline")
    end

    _report_timeout_expiry->>_log_timeout_expiry: _log_timeout_expiry(pid, configured_timeout, mode)
    _report_timeout_expiry->>_emit_timeout_event: _emit_timeout_event(observation, pid, configured_timeout, mode)
    _emit_timeout_event->>_StageObservation: emit("timeout", _EventDetails(...))
    _wait_for_exit_code_within_timeout-->>_execute_subprocess: raise TimeoutError
    _execute_subprocess-->>SafeCmd_run: propagate TimeoutError
    SafeCmd_run-->>User: TimeoutExpired with partial output

Loading

File-Level Changes

Change	Details	Files
Introduce structured timeout/teardown observability helpers and types for subprocess timeout paths, including safe logging and observe-event emission that cannot mask failures.	Add cuprum.timeout module logger and TimeoutMode type to distinguish elapsed deadlines from non-positive immediate expiries. Implement helpers to emit structured timeout and teardown diagnostics as log records with cuprum* extras, swallowing logging failures. Implement helpers to emit timeout and teardown_error observe events via _StageObservation using shared _EventDetails, swallowing synchronous hook failures. Add _report_teardown_drain_failure to pair teardown diagnostics across logging and observe channels and export new helpers from the module.	cuprum/_subprocess_timeout.py
Refactor subprocess execution timeout handling to use caller-owned asyncio.timeout deadlines, integrate new telemetry helpers, and ensure cleanup cancels and drains consumers and stdin writers exactly once without masking primary exceptions.	Introduce _report_timeout_expiry to pair timeout diagnostics across logging and observe events using _TimeoutMode. Update _wait_for_exit_code_within_timeout to special-case non-positive timeouts for immediate expiry, wrap _wait_for_exit_code in asyncio.timeout for positive deadlines, and emit paired timeout diagnostics before re-raising TimeoutError. Extend _drain_stream_consumers to report unexpected drain errors via _report_teardown_drain_failure while still absorbing them so they cannot mask timeouts or cancellations. Add _run_subprocess_without_streams to encapsulate the direct (no capture/echo) path with proper stdin writer cancellation and draining on any failure. Rework _run_subprocess_with_streams and _execute_subprocess to use the new drain helper, propagate observation to drains, and route the direct path through _run_subprocess_without_streams. Export new timeout/telemetry helpers from the execution module.	cuprum/_subprocess_execution.py
Extend ExecEvent and pipeline event plumbing to carry timeout-specific fields and new phases, and ensure observe emission uses these fields consistently.	Add timeout and teardown_error phases to ExecPhase and document them along with their ancillary nature and preserved exit/TimeoutExpired behaviour. Extend ExecEvent and _EventDetails with timeout_s and timeout_mode fields and update _StageObservation.emit to populate them. Update adapter test support factory to initialize and cast the new timeout fields. Ensure pid is documented as present for all phases except plan.	cuprum/events.py cuprum/_pipeline_types.py cuprum/unittests/_adapter_test_support.py
Update metrics adapter to treat timeout and teardown_error as counter-based phases and factor common counter dispatch logic.	Introduce _COUNTER_METRICS mapping phases to metric names, including new cuprum_timeouts_total and cuprum_teardown_errors_total. Refactor MetricsHook.call to use _COUNTER_METRICS for simple per-event counters and keep stdin/exit special handling. Document the new timeout and teardown error metrics in MetricsHook docstring. Add tests that verify timeout and teardown_error events increment the appropriate counters only.	cuprum/adapters/metrics_adapter.py cuprum/unittests/test_metrics_adapter.py
Extend tracing adapter to record ancillary timeout and teardown_error events as span events with stable attributes while leaving the span open.	Define _SPAN_FIELDS to include line, operation, error_type, note, timeout_s, and timeout_mode. Update TracingHook.call to treat stdin_error, timeout, and teardown_error phases as span events. Update _record_span_event to include timeout-specific fields in event attributes. Replace stdin_error-specific test with a parametrized test that validates stdin_error, timeout, and teardown_error span events and confirms spans remain open.	cuprum/adapters/tracing_adapter.py cuprum/unittests/test_tracing_adapter.py
Harden observe-hook task cleanup for single-command executions to aggregate async hook failures with primary errors using shared pipeline cleanup logic.	Export _drain_tasks_during_cleanup from _pipeline_internals and require a caller-supplied finalization message. Update pipeline cleanup (_finalize_pipeline_execution, _run_pipeline) to pass _PIPELINE_FINALIZATION_ERROR into _drain_tasks_during_cleanup and adjust tests accordingly. Update _execute_with_hooks in sh.py to use _drain_tasks_during_cleanup for both cancellation and failure paths, aggregating hook-task failures into BaseExceptionGroup with a command-specific finalization message while preserving TimeoutExpired/CancelledError or other run errors. Add tests that async observe-hook failures on timeout or cancellation are aggregated rather than masking the primary error.	cuprum/_pipeline_internals.py cuprum/sh.py cuprum/unittests/test_cqrs_hook_behaviour.py cuprum/unittests/test_observe_async_hook_failure.py
Enhance timeout-related unit tests and shared helpers to cover logging, observe events, telemetry pairing, and reuse doubles across suites.	Move timeout-related process and execution doubles into a new shared _timeout_test_helpers module and update existing timeout cleanup tests to import from it. Add structured logging tests for timeout expiry (elapsed and non-positive) and teardown drain failures, including behaviour when logging fails. Add observe-side timeout tests for elapsed and non-positive expiry, teardown_error events, and behaviour when observation emit raises. Add property-based tests that assert timeout and teardown diagnostics are paired consistently across logging and observe channels for generated inputs.	cuprum/unittests/test_subprocess_timeout.py cuprum/unittests/_timeout_test_helpers.py cuprum/unittests/test_subprocess_timeout_logging.py cuprum/unittests/test_subprocess_timeout_observe.py cuprum/unittests/test_timeout_telemetry_pairing.py
Add documentation and doc-contract tests capturing ASYNC lint policy, timeout observability, and wait-helper ADR addendum, and ensure users’ guide reflects timeout behaviour and events.	Extend developers’ guide with Ruff ASYNC (flake8-async) policy section describing selected rules, scoped suppressions, and per-file ignores, and add subprocess timeout observability section describing timeout/teardown_error events, cuprum.timeout logs, metrics, and tracing behaviour. Update ADR-007 with an addendum documenting wait-helper decomposition, non-positive fast path, drain-once semantics, no-orphan invariant, and timeout observability, ensuring public API behaviour is unchanged. Update users’ guide timeout section to describe non-positive timeout immediate expiry and cleanup semantics, and structured events section to document timeout and teardown_error events and timeout_mode semantics. Add async_timeout_docs test module that asserts required phrases are present in docs and ADR for ASYNC policy, timeout observability, users’ guide timeout contract, event docs, and ADR-007 wait-helper addendum.	docs/developers-guide.md docs/adr-007-subprocess-execution-module-boundaries.md docs/users-guide.md cuprum/unittests/test_async_timeout_docs.py

---

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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

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

Please try again later or upgrade to continue using Sourcery

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a17a1241e6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cuprum/_pipeline_types.py (1)

71-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use ExecPhase in emit().

Import it alongside ExecEvent and remove the duplicated Literal[...] declaration. The two declarations contain the same nine phases.

🤖 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/_pipeline_types.py` around lines 71 - 83, Update emit() to accept the
shared ExecPhase type instead of its duplicated Literal declaration, and import
ExecPhase alongside ExecEvent. Preserve the existing nine-phase API values while
removing the redundant inline declaration.

🤖 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/_pipeline_results.py`:
- Around line 51-61: Guard each stage’s obs.emit call in
_emit_timeout_exit_events with per-stage best-effort suppression of Exception
and asyncio.CancelledError, ensuring one hook failure does not replace
TimeoutExpired or stop later exit emissions; move asyncio to runtime imports as
needed. No direct change is required in cuprum/_pipeline_internals.py lines
319-328 once this guard is added; confirm the original TimeoutExpired is still
re-raised. Add coverage for a synchronous exit hook failure during timeout,
verifying TimeoutExpired reaches the caller and every stage emits exit.

In `@cuprum/_timeout_reporting.py`:
- Around line 234-246: Update the timeout reporting loop around
_report_timeout_expiry so asyncio.CancelledError from process.pid access is
suppressed at the pipeline boundary, while preserving propagation of the
original TimeoutExpired. Ensure the guard covers the entire
per-observation/process reporting operation, including evaluating process.pid,
rather than relying only on inner telemetry suppression.

In `@cuprum/adapters/tracing_adapter.py`:
- Around line 253-254: Update event handling in
cuprum/adapters/tracing_adapter.py lines 253-254 so teardown_error uses
lifecycle handling that supports either event order and records the failure
without losing valid lifecycle data. Finalize and remove active spans for
teardown-only executions in lines 324-330, while safely handling teardown_error
after exit; replace the assertion requiring ancillary events to leave spans
open. Add tests in cuprum/unittests/test_tracing_adapter.py lines 147-234
covering both final teardown_error and teardown_error-after-exit orderings.

In `@cuprum/events.py`:
- Around line 155-158: Define and export a public TimeoutMode alias in
cuprum.events covering the two supported literal values, then annotate
ExecEvent.timeout_mode and _EventDetails.timeout_mode with it. Update
_timeout_reporting._TimeoutMode to alias the public TimeoutMode instead of
defining a separate type, preserving existing behavior.

In `@cuprum/sh.py`:
- Line 26: Move the _drain_tasks_during_cleanup definition from its current
module into _observability.py beside _wait_for_exec_hook_tasks, export it there,
and update imports in cuprum/sh.py, cuprum/_pipeline_internals.py, and the unit
tests to use the new location.

In `@cuprum/unittests/test_pipeline_timeout_telemetry.py`:
- Around line 188-191: Update the ordering assertion in the pipeline timeout
telemetry test to group events by pid and, for each stage with a timeout, verify
that its timeout event occurs before the corresponding exit event. Replace the
global phases.index checks while preserving the failure context in assertion
messages.

In `@docs/users-guide.md`:
- Around line 999-1010: Update the “Ancillary span events” documentation to
state that the execution’s open span remains unended and unmarked until an exit
event is emitted, and that exit closes it when present; remove the implication
that exit always follows teardown_error. Preserve the existing event-field and
exec_id correlation details.
- Around line 397-410: Update docs/users-guide.md lines 397-410 to document
cuprum_operation as "drain" instead of "teardown", keeping the teardown_error
event description consistent. In cuprum/unittests/test_async_timeout_docs.py
lines 232-255, strengthen the documentation assertion to verify that
cuprum_operation is specifically associated with "drain", rather than asserting
the terms independently.

---

Outside diff comments:
In `@cuprum/_pipeline_types.py`:
- Around line 71-83: Update emit() to accept the shared ExecPhase type instead
of its duplicated Literal declaration, and import ExecPhase alongside ExecEvent.
Preserve the existing nine-phase API values while removing the redundant inline
declaration.

🪄 Autofix

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: 4e9f58ae-8daf-49c8-b362-82ea370f38a6

📥 Commits

Reviewing files that changed from the base of the PR and between 7f6ec92 and 7b10be8.

📒 Files selected for processing (34)

• CHANGELOG.md
• cuprum/_pipeline_internals.py
• cuprum/_pipeline_results.py
• cuprum/_pipeline_types.py
• cuprum/_subprocess_execution.py
• cuprum/_subprocess_timeout.py
• cuprum/_subprocess_wait.py
• cuprum/_timeout_reporting.py
• cuprum/adapters/metrics_adapter.py
• cuprum/adapters/tracing_adapter.py
• cuprum/events.py
• cuprum/sh.py
• cuprum/unittests/__snapshots__/test_adapter_projection.ambr
• cuprum/unittests/__snapshots__/test_maturin_build.ambr
• cuprum/unittests/_adapter_test_support.py
• cuprum/unittests/_timeout_test_helpers.py
• cuprum/unittests/test_adapter_projection.py
• cuprum/unittests/test_async_timeout_docs.py
• cuprum/unittests/test_cqrs_hook_behaviour.py
• cuprum/unittests/test_metrics_adapter.py
• cuprum/unittests/test_observe.py
• cuprum/unittests/test_observe_async_hook_failure.py
• cuprum/unittests/test_pipeline_timeout_telemetry.py
• cuprum/unittests/test_public_api.py
• cuprum/unittests/test_subprocess_timeout.py
• cuprum/unittests/test_subprocess_timeout_logging.py
• cuprum/unittests/test_subprocess_timeout_observe.py
• cuprum/unittests/test_subprocess_timeout_properties.py
• cuprum/unittests/test_timeout_telemetry_pairing.py
• cuprum/unittests/test_tracing_adapter.py
• docs/adr-007-subprocess-execution-module-boundaries.md
• docs/cuprum-design.md
• docs/developers-guide.md
• docs/users-guide.md

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate any existing public alias for the timeout modes.
rg -nP --type=py 'TimeoutMode|elapsed_deadline|non_positive_immediate' -g '!**/unittests/**'

# Map the public event surface to check what is already exported.
fd -t f 'events.py' -p cuprum --exec ast-grep outline {} --items all

# Find consumers that compare timeout_mode against a string literal.
rg -nP --type=py -C3 'timeout_mode\s*(==|!=|in)\s'

Repository: leynos/cuprum

Length of output: 151

---

🏁 Script executed:

set -u

printf '%s\n' '--- target files ---'
sed -n '1,230p' cuprum/events.py
printf '%s\n' '--- pipeline types ---'
sed -n '1,100p' cuprum/_pipeline_types.py
printf '%s\n' '--- timeout reporting ---'
sed -n '1,100p' cuprum/_timeout_reporting.py
printf '%s\n' '--- timeout identifiers and exports ---'
rg -n -P --type=py 'TimeoutMode|elapsed_deadline|non_positive_immediate|timeout_mode' cuprum || true
printf '%s\n' '--- package exports ---'
fd -t f -d 2 -e py . cuprum | sort | xargs -r rg -n 'events|ExecEvent|__all__' || true

Repository: leynos/cuprum

Length of output: 50369

---

🏁 Script executed:

python3 - <<'PY'
import ast
from pathlib import Path

root = Path("cuprum")
files = tuple(root.rglob("*.py"))
mode_values = {"elapsed_deadline", "non_positive_immediate"}
string_uses = []
aliases = []
exports = {}

for path in files:
    tree = ast.parse(path.read_text(), filename=str(path))
    for node in ast.walk(tree):
        if isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name):
            text = ast.unparse(node.annotation)
            if "timeout_mode" in node.target.id or node.target.id in {"_TimeoutMode", "TimeoutMode"}:
                aliases.append((str(path), node.lineno, node.target.id, text))
        if isinstance(node, ast.Assign):
            for target in node.targets:
                if isinstance(target, ast.Name) and target.id in {"_TimeoutMode", "TimeoutMode"}:
                    aliases.append((str(path), node.lineno, target.id, ast.unparse(node.value)))
        if isinstance(node, ast.Call) and isinstance(node.func, ast.Name) and node.func.id == "ExecEvent":
            for keyword in node.keywords:
                if keyword.arg == "timeout_mode":
                    string_uses.append((str(path), node.lineno, ast.unparse(keyword.value)))
    for node in tree.body:
        if isinstance(node, ast.Assign):
            for target in node.targets:
                if isinstance(target, ast.Name) and target.id == "__all__":
                    try:
                        exports[str(path)] = ast.literal_eval(node.value)
                    except Exception:
                        exports[str(path)] = ast.unparse(node.value)

print("aliases:")
for item in aliases:
    print(item)
print("timeout_mode annotations/call values:")
for item in string_uses:
    print(item)
print("events exports:", exports.get("cuprum/events.py"))
print("mode literals used in ExecEvent calls:",
      sorted({value for _, _, value in string_uses if value in {repr(v) for v in mode_values}}))
PY

printf '%s\n' '--- public package exports and relevant internal tails ---'
sed -n '45,110p' cuprum/__init__.py
sed -n '85,120p' cuprum/_pipeline_types.py
sed -n '250,285p' cuprum/_timeout_reporting.py

Repository: leynos/cuprum

Length of output: 4235

---

🏁 Script executed:

python3 - <<'PY'
import re
from pathlib import Path

mode_pattern = re.compile(r'"(elapsed_deadline|non_positive_immediate)"')
for name in ("cuprum/events.py", "cuprum/_pipeline_types.py", "cuprum/_timeout_reporting.py"):
    text = Path(name).read_text()
    print(name)
    for line_no, line in enumerate(text.splitlines(), 1):
        if "TimeoutMode" in line or "timeout_mode" in line or mode_pattern.search(line):
            print(f"{line_no}: {line}")

print("\nmode literals outside tests:")
for path in sorted(Path("cuprum").rglob("*.py")):
    if "unittests" in path.parts:
        continue
    for line_no, line in enumerate(path.read_text().splitlines(), 1):
        values = mode_pattern.findall(line)
        if values:
            print(f"{path}:{line_no}: {values}")

print("\npublic alias/export indicators:")
events = Path("cuprum/events.py").read_text()
print("TimeoutMode declaration:", bool(re.search(r"(?m)^type TimeoutMode\s*=", events)))
print("__all__ contains TimeoutMode:",
      bool(re.search(r'(?m)^\s*"TimeoutMode",?', events)))
print("_timeout_reporting imports from events:",
      bool(re.search(r"from cuprum\.events import[^\n]*TimeoutMode", Path("cuprum/_timeout_reporting.py").read_text())))
PY

printf '%s\n' '--- project Python target and typing conventions ---'
rg -n -C2 'requires-python|python_version|type [A-Za-z_]+ =' pyproject.toml setup.cfg tox.ini Makefile 2>/dev/null || true

Repository: leynos/cuprum

Length of output: 2292

---

Export and reuse a public TimeoutMode alias.

Define TimeoutMode in cuprum.events for the two supported literals, add it to __all__, and use it for ExecEvent.timeout_mode and _EventDetails.timeout_mode. Replace _timeout_reporting._TimeoutMode with an alias to the public type.

🤖 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/events.py` around lines 155 - 158, Define and export a public
TimeoutMode alias in cuprum.events covering the two supported literal values,
then annotate ExecEvent.timeout_mode and _EventDetails.timeout_mode with it.
Update _timeout_reporting._TimeoutMode to alias the public TimeoutMode instead
of defining a separate type, preserving existing behavior.

Source: Coding guidelines

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the teardown diagnostic operation value consistent.

Replace "teardown" with "drain" for cuprum_operation. The guide states
that logging and observe events use shared values, while the teardown_error
event schema defines its operation as "drain". Add an assertion that binds
cuprum_operation specifically to "drain".

• docs/users-guide.md#L397-L410: document
  cuprum_operation as "drain".
• cuprum/unittests/test_async_timeout_docs.py#L232-L255: assert the
  cuprum_operation and "drain" association, not only independent terms.

Triage: [type:docstyle]

📍 Affects 2 files

• docs/users-guide.md#L397-L410 (this comment)
• cuprum/unittests/test_async_timeout_docs.py#L232-L255

🤖 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/users-guide.md` around lines 397 - 410, Update docs/users-guide.md lines
397-410 to document cuprum_operation as "drain" instead of "teardown", keeping
the teardown_error event description consistent. In
cuprum/unittests/test_async_timeout_docs.py lines 232-255, strengthen the
documentation assertion to verify that cuprum_operation is specifically
associated with "drain", rather than asserting the terms independently.

Source: Coding guidelines

[leynos]

@coderabbitai Have the following failed checks now been resolved?

If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures.

Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat o11y, code safety, documentation and validation coverage as in scope).

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
User-Facing Documentation	⚠️ Warning	The users' guide clearly documents timeout modes, log fields, observe phases, pipeline reporting, metrics, and tracing, but no n+1 migration note covers this new functionality.	Add an Unreleased Added/Changed entry to CHANGELOG.md that signposts the new timeout and teardown telemetry and states that adoption is additive.
Developer Documentation	⚠️ Warning	The developer guide still states that ExecPhase has seven phases, but the code defines nine including timeout and teardown_error; the design boundary section also omits _timeout_reporting.	Update docs/developers-guide.md and docs/cuprum-design.md with the nine-phase mapping, _timeout_reporting boundary, and new direct-path cleanup helpers.
Concurrency And State	⚠️ Warning	Cleanup awaits in _run_subprocess_with_streams and _run_pipeline are not shielded; cancellation during termination or draining can leave tasks/processes unfinished. Tests cancel only before cleanup.	Shield or otherwise finish process and task cleanup before propagating cancellation. Add deterministic tests that cancel during termination, stream draining, and hook-task cleanup, then assert no pending tasks or live children.

[codescene-access]

No quality gates enabled for this code.