Skip to content

📝 CodeRabbit Chat: Implement requested code changes - #172

Open
coderabbitai[bot] wants to merge 12 commits into
mainfrom
coderabbitai/chat/27b89aa
Open

📝 CodeRabbit Chat: Implement requested code changes#172
coderabbitai[bot] wants to merge 12 commits into
mainfrom
coderabbitai/chat/27b89aa

Conversation

@coderabbitai

@coderabbitai coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Code changes was requested by @leynos.

The following files were modified:

  • cuprum/unittests/test_rust_streams.py

leynos and others added 12 commits June 16, 2026 02:16
`rust_consume_stream` is shipped, public in `cuprum/_streams_rs.py`,
tested, and documented, yet no production code routes a consume through
it — unlike the pump side, which has a `_pump_stream_dispatch` with a
Rust-then-Python fallback. ADR-002 frames consume-routing as deferred,
evidence-gated Phase 2 work (after the Phase 1 measurement schema), so
the symbol's status was ambiguous to readers.

Of the two resolutions the issue offers, take the annotation path —
wiring a `_consume_stream_dispatch` now would bypass the ADR's
measurement gate:

- Annotate `rust_consume_stream` (module and function docstrings) as
  "implemented but not yet integrated", pointing at ADR-002 Phase 2.
- Reflect the status in `docs/users-guide.md` so the private API is not
  mistaken for a wired bridge.
- Record the decision in `docs/adr-002-additional-rust-components.md`
  under the Phase 2 heading.
- Add a guard test asserting the docstring carries the marker and that
  no production module references the symbol; wiring the dispatcher
  later must remove the marker and the guard together.
Record the tee hot-path baseline findings in the rust consume-stream
component documentation. Keep the current branch's annotation intact:
`rust_consume_stream` is implemented and tested, but production dispatch
must still wait for the Phase 2 dispatcher, fallback path, and parity
property tests.

Clarify the design diagrams and backend-selection text so they no longer
imply that stdout or stderr consumption is routed through Rust today.
Clarify that the tee profiling baseline provides hotspot evidence for
Phase 2, not acceptance of the consume dispatcher. Keep the 20% median
wall-time gate tied to a future Rust-versus-Python dispatcher benchmark.
Use a NumPy-style `Notes` section for the `rust_consume_stream`
status note, keeping the helper documentation consistent with the rest of
its docstring.

Switch the not-integrated guard from raw text matching to AST symbol
reference detection so comments and docstrings do not create false
positives when checking production modules.
Split the consume-stream status guard so the docstring marker and the
production-reference scan are checked independently.

Add maintainer documentation for the current `rust_consume_stream` state:
it is implemented, tested, and exported, but production consume dispatch
remains deferred to ADR-002 Phase 2 until dispatcher wiring, Python
fallback, and parity/property coverage land.
Add roadmap phases 5-8 translating the tee hot-path profiling baseline and
the issue #90/#127 follow-up into outcome-oriented work:

- Phase 5 lands the Rust-free wins first (read-size plateau bump and lean
  per-line observe-hook emission) so the consume dispatcher must beat a tuned
  Python baseline.
- Phase 6 hardens the PyO3 boundary (#90), establishes the 20% wall-time
  acceptance gate, wires _consume_stream_dispatch, and records the outcome.
- Phase 7 scopes raw-sink echo/tee acceleration, profiling first to exclude
  kernel-bound PTY cost.
- Phase 8 collects deferred reliability work (Rust pump FD close race, perf
  frame attribution) and the native-orchestration go/no-go decisions.

Each phase carries a falsifiable GIST idea; tasks cite ADR-002, the design
doc, the profiling baseline, and the relevant issues.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Handle AST scan read and parse failures explicitly in the production
reference guard, and add coverage for invalid candidate modules.

Update the consume-stream documentation to use current `echo=True`
terminology and clarify that only pump-side routing has Rust backing today.
Replace the awkward consume-routing wording in ADR-002 and tighten the
design guide so it says final stream consumption still uses the Python
path until the Phase 2 consume dispatcher is wired and tested.
Avoid implying that a consume dispatcher is already wired for line-callback
streams. State that stream consumption stays on the Python pathway until the
Phase 2 consume dispatcher exists.
Use the specific module-reference scan error in the guard test and
tighten the consume-stream documentation wording.

Expand the consume-dispatch roadmap gate so it covers fallback
regressions, parity before routing, and outcome-specific documentation
dependencies.
@coderabbitai
coderabbitai Bot requested a review from leynos June 16, 2026 18:59

@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.

Sorry @coderabbitai[bot], you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jun 16, 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: 07c6f79f-64c7-4fb0-bb7d-920fc7785558

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.

@lodyai
lodyai Bot force-pushed the issue-127-consume-stream-dispatch branch from d104210 to 382d120 Compare June 23, 2026 22:29
Base automatically changed from issue-127-consume-stream-dispatch to main June 25, 2026 22:21
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