Skip to content

Centralize streaming inbound assertions; add ExecPlan doc and tests#501

Merged
leynos merged 8 commits intomainfrom
update-docs-streaming-requests-yqxepp
Mar 18, 2026
Merged

Centralize streaming inbound assertions; add ExecPlan doc and tests#501
leynos merged 8 commits intomainfrom
update-docs-streaming-requests-yqxepp

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented Mar 14, 2026

## Summary - Extends streaming test improvements to include streaming inbound tests, centralizes assertion helpers in fixtures, and adds an ExecPlan document to close the streaming-requests documentation loop. Also updates related docs and test scaffolding to reflect the streaming inbound path and budgeting.

Changes

  • Add new ExecPlan doc: docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
  • Tests and fixtures for streaming requests:
    • tests/features/streaming_request.feature
    • tests/fixtures/streaming_request.rs
    • tests/scenarios/streaming_request_scenarios.rs
    • tests/steps/streaming_request_steps.rs
    • tests/scenarios/mod.rs updated to include streaming_request_scenarios
    • tests/fixtures/mod.rs updated to expose streaming_request
    • tests/steps/mod.rs updated to include streaming_request_steps
  • Refactor: centralize common assertion helpers in streaming tests to reduce duplication, with shared helpers in tests/fixtures/streaming_request.rs (e.g., assert_field_eq, NextChunk classification) reused by streaming_request_steps.rs and scenarios.
  • Code/doc tweaks to reflect streaming inbound path and budgeting:
    • src/app/builder/config.rs: add docs for memory_budgets
    • src/app/builder/protocol.rs: clarify MessageAssembler integration timing
  • Documentation updates to harmonize with ADR-0002:
    • docs/generic-message-fragmentation-and-re-assembly-design.md
    • docs/multi-packet-and-streaming-responses-design.md
    • docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md
    • docs/users-guide.md
  • Roadmap update: docs/roadmap.md to reflect 8.6.1–8.6.3 gating status

Rationale

  • The repository already contains ADR-0002 and related design docs; this change closes the documentation cycle by adding an ExecPlan and by introducing focused behavioural tests for streaming inbound paths. It also includes a targeted refactor to reduce duplication in assertion logic across streaming tests and aligns public docs with the current implementation surface.

Validation and acceptance criteria

  • The ExecPlan document exists at docs/execplans/8-6-1-update-documentation-for-streaming-requests.md and is coherent with ADR 0002.
  • It clearly defines the staged approach (A–E) and gating requirements to mark 8.6.1–8.6.3 done.
  • All reference materials and cross-links point to the correct design documents.
  • Tests for streaming inbound paths exist and compile; no runtime API changes are introduced beyond documentation and test scaffolding.

Impacted artifacts

  • docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
  • docs/generic-message-fragmentation-and-re-assembly-design.md
  • docs/multi-packet-and-streaming-responses-design.md
  • docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md
  • docs/users-guide.md
  • docs/roadmap.md
  • src/app/builder/config.rs
  • src/app/builder/protocol.rs
  • tests/features/streaming_request.feature
  • tests/fixtures/streaming_request.rs
  • tests/scenarios/mod.rs
  • tests/scenarios/streaming_request_scenarios.rs
  • tests/steps/mod.rs
  • tests/steps/streaming_request_steps.rs
  • tests/fixtures/mod.rs

📎 Task: https://www.devboxer.com/task/70376dcf-86a9-4729-becb-3a2a78f480e3

…ion closure

Add a comprehensive draft ExecPlan outlining the closing documentation phase for roadmap items 8.6.1 to 8.6.3. This new document audits existing materials, aligns multi-document claims, specifies constraints, risks, and test coverage gaps, and plans targeted edits and validation steps to finalize streaming request body documentation, MessageAssembler usage, and memory budgets guidance. It ensures consistency across design docs, user guide, tests, and enforcement of quality gates before marking roadmap items complete.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Mar 14, 2026

Reviewer's Guide

Adds an ExecPlan documenting completion criteria for streaming requests documentation work, clarifies how streaming requests, MessageAssembler, and memory budgets interact in multiple docs and Rustdoc, and introduces a focused BDD suite plus fixtures for streaming inbound request bodies with centralized assertion helpers.

Sequence diagram for inbound streaming request pipeline and budgets

sequenceDiagram
    actor Client
    participant TransportCodec
    participant FragmentAdapter
    participant MessageAssembler
    participant InboundPipeline
    participant Handler

    Client->>TransportCodec: send_request()
    TransportCodec->>FragmentAdapter: decode_transport_frame()
    FragmentAdapter-->>TransportCodec: reassembled_packet

    TransportCodec->>MessageAssembler: deliver_protocol_packet()
    MessageAssembler-->>InboundPipeline: assembled_message_or_stream()

    InboundPipeline->>InboundPipeline: enforce_per_frame_budgets()
    InboundPipeline->>InboundPipeline: apply_soft_pressure_at_80_percent()
    InboundPipeline->>InboundPipeline: abort_on_hard_cap_exceeded()

    alt Buffered_request
        InboundPipeline->>Handler: handle(buffered_request)
    else Streaming_request
        InboundPipeline->>Handler: handle(RequestParts, RequestBodyStream)
    end

    Handler-->>Client: send_response()
Loading

Class diagram for streaming request test fixtures and centralized helpers

classDiagram
    class StreamingRequestFixture {
        +setup_inbound_stream()
        +send_body_chunk(bytes)
        +finish_request()
        +assert_field_eq(field, expected, actual)
        +classify_next_chunk(chunk) NextChunk
    }

    class NextChunk {
        <<enumeration>>
        +Metadata
        +Body
        +End
        +Error
    }

    class StreamingRequestSteps {
        +given_streaming_request_fixture(fixture StreamingRequestFixture)
        +when_handler_consumes_body()
        +then_chunks_arrive_in_order()
        +then_backpressure_is_applied()
        +then_errors_propagate_to_handler()
    }

    class StreamingRequestScenarios {
        +streaming_body_happy_path()
        +streaming_body_backpressure()
        +streaming_body_error_propagation()
    }

    StreamingRequestSteps --> StreamingRequestFixture
    StreamingRequestScenarios --> StreamingRequestFixture
Loading

File-Level Changes

Change Details Files
Clarify fragmentation, MessageAssembler composition, streaming request API, and memory budget behaviour across design and user docs, and mark roadmap items 8.6.1–8.6.3 as complete.
  • Explain operational hand-offs between transport codec, FragmentAdapter, MessageAssembler, and handler-visible request types, including when bodies are buffered vs streamed.
  • Document concrete RequestParts, RequestBodyStream, StreamingBody, and RequestBodyReader shapes and how handlers opt into streaming while preserving the default buffered path.
  • Describe the fixed inbound ordering (transport, optional fragmentation, MessageAssembler, handler) and how per-frame rejection, soft-pressure pacing, and hard-cap abort share per-connection memory budgets, including defaults derived from buffer_capacity.
  • Update capability-maturity language to reflect MessageAssembler and three-tier memory budget model as implemented features, not future intent, and align user guide explanations and examples.
  • Mark roadmap 8.6.1–8.6.3 as done and tighten Rustdoc on with_message_assembler and memory_budgets to reflect actual integration points.
docs/generic-message-fragmentation-and-re-assembly-design.md
docs/multi-packet-and-streaming-responses-design.md
docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md
docs/users-guide.md
docs/roadmap.md
src/app/builder/config.rs
src/app/builder/protocol.rs
Introduce an ExecPlan documenting scope, constraints, risks, validation steps, and acceptance criteria for closing the 8.6 streaming-requests documentation phase.
  • Add a detailed execution plan that defines purpose, constraints, tolerances, risks, progress stages A–E, and explicit decision log for the 8.6 documentation work.
  • Describe existing documentation, implementation, and test surfaces for streaming requests, MessageAssembler, and memory budgets as context for the work.
  • Specify concrete commands and Makefile targets for validation (fmt, lint, tests, doctests, markdownlint, nixie) and gating roadmap checkbox updates.
  • Define validation and acceptance criteria ensuring docs, tests, and roadmap status are consistent and idempotent recovery steps for failed gates.
docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
Add a focused BDD suite and fixture for streaming inbound request bodies, centralizing assertion and helper logic.
  • Create a StreamingRequestWorld fixture that owns a single-threaded Tokio runtime, request body channel sender/stream, and accumulates collected body bytes, chunk counts, last error, and back-pressure observations.
  • Provide helper methods to create channels via body_channel, send chunks or io::Error into the RequestBodyStream (with optional timeout to detect back-pressure), drain via StreamingBody -> RequestBodyReader (AsyncRead), or drain the stream directly while classifying chunks and capturing error kinds.
  • Define parsing and conversion helpers for human-readable io::ErrorKind values used in feature steps, and shared assert_field_eq plus NextChunk classification to normalize assertions across scenarios.
  • Wire the fixture into new rstest-bdd steps and scenarios that validate AsyncRead adapter behaviour, bounded channel back-pressure, and that stream errors reach consumers, and register new modules in tests/fixtures/mod.rs, tests/steps/mod.rs, and tests/scenarios/mod.rs.
tests/fixtures/streaming_request.rs
tests/features/streaming_request.feature
tests/steps/streaming_request_steps.rs
tests/scenarios/streaming_request_scenarios.rs
tests/fixtures/mod.rs
tests/steps/mod.rs
tests/scenarios/mod.rs

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: afc174c0-d323-42fe-aa1a-a37a09a94fd8

📥 Commits

Reviewing files that changed from the base of the PR and between f866234 and 7c68268.

📒 Files selected for processing (2)
  • tests/fixtures/streaming_request.rs
  • tests/steps/streaming_request_steps.rs

Summary by CodeRabbit

  • Documentation

    • Added an ExecPlan for streaming requests; expanded fragmentation/re‑assembly, memory‑budget semantics, user‑guide clarifications and roadmap updates.
  • New Features

    • Published concrete streaming request surface: request parts/streams, handler signatures, response streaming helpers and streaming error/stream types.
  • Tests

    • Added BDD scenarios, test fixtures and step definitions covering AsyncRead adapter, bounded back‑pressure and stream error propagation.

Walkthrough

Document the completion of streaming requests and shared message assembly: add an ExecPlan and update design docs, roadmap and user guide; extend builder doc-comments; introduce a BDD test suite (feature, fixtures, scenarios, steps) that exercises streaming request bodies. No runtime public API changes in this diff.

Changes

Cohort / File(s) Summary
ExecPlan & Roadmap
docs/execplans/8-6-1-update-documentation-for-streaming-requests.md, docs/roadmap.md
Add comprehensive ExecPlan for staged documentation closure, validation gates, audits and artifact generation; mark three 8.6 roadmap items as completed.
Design Documents
docs/generic-message-fragmentation-and-re-assembly-design.md, docs/multi-packet-and-streaming-responses-design.md, docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md
Expand transport vs protocol hand-offs; formalise unified memory-budget model (soft-pressure at 80% and hard-cap abort); publish concrete inbound streaming API shapes and helpers (Response::with_channel*), add WireframeError and ResponseStream concepts; clarify terminator/correlation semantics and assembler integration.
Users Guide
docs/users-guide.md
Clarify when metadata splits occur with MessageAssembler, describe streaming vs buffered handler paths, document memory_budget defaults and protection behaviour, and explain routing/correlation visibility during streaming.
Code Documentation
src/app/builder/config.rs, src/app/builder/protocol.rs
Add extended doc-comment to memory_budgets() describing shared inbound assembly budget semantics; expand with_message_assembler() docs to specify assembler placement after transport reassembly and before handler dispatch.
BDD Feature File
tests/features/streaming_request.feature
Add three feature scenarios covering AsyncRead adapter, bounded-channel back-pressure, and stream error propagation.
Test Fixtures
tests/fixtures/mod.rs, tests/fixtures/streaming_request.rs
Expose and implement streaming_request fixture module; add StreamingRequestWorld with runtime-backed channel creation, send/drain APIs, error injection, and assertion helpers for streaming request tests.
Scenarios
tests/scenarios/mod.rs, tests/scenarios/streaming_request_scenarios.rs
Register and add three rstest-bdd scenarios referencing the new feature file and using StreamingRequestWorld fixtures (placeholder bodies).
Step Definitions
tests/steps/mod.rs, tests/steps/streaming_request_steps.rs
Register and implement BDD step functions that map feature steps to fixture operations (channel setup, send chunk(s), send with timeout, inject error, drain/read, and assertions).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client
    participant Transport as Transport/Codec
    participant FragmentAdapter
    participant MessageAssembler
    participant Wireframe
    participant Handler

    Client->>Transport: send logical request (may fragment)
    Transport->>FragmentAdapter: deliver transport packet(s)
    FragmentAdapter->>MessageAssembler: reassembled transport frames
    MessageAssembler->>Wireframe: classified frame (first/continuation) + body stream or buffered body
    alt streaming path
        Wireframe->>Handler: invoke handler with RequestParts + RequestBodyStream/StreamingBody
        Handler->>Wireframe: consume stream (back-pressure applies against memory_budgets)
    else buffered path
        Wireframe->>Handler: invoke handler with buffered RequestParts and complete body
    end
    Note over MessageAssembler,Wireframe: memory_budgets enforce per-frame reject, soft-pressure (~80%), hard-cap abort (connection)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

Stitch frames, stitch docs, stitch tests into streams,
Handlers keep metadata while body data streams,
Budgets sigh at eighty then stand firm at the cap,
ExecPlan ticks gates; scenarios map the tap,
Mark the roadmap checked — let the CI dreams gleam.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarises the two primary changes: centralising streaming inbound test assertions and adding an ExecPlan document to close the documentation phase.
Description check ✅ Passed The description thoroughly explains the scope, rationale, and validation criteria, mapping directly to the changeset across docs, tests, and fixtures.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 update-docs-streaming-requests-yqxepp
📝 Coding Plan
  • Generate coding plan for human review comments

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

codescene-delta-analysis[bot]

This comment was marked as outdated.

…h memory budgets

- Completed documentation updates for streaming request body APIs and message assembler integration.
- Implement inbound streaming request bodies exposing RequestParts and RequestBodyStream or StreamingBody.
- Enforce shared per-connection memory budgets with per-frame rejection, soft read pacing, and hard abort.
- Added builder support for configuring memory budgets and message assembler hook.
- Introduced comprehensive behavioural tests using rstest-bdd for back-pressure, error propagation, and AsyncRead adapter correctness.
- Enhanced public API guidance ensuring protocol authors can adopt streaming incrementally and preserve ergonomic buffered requests.

This allows handlers to consume large inbound payloads incrementally while preserving back-pressure and reliable error handling.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos leynos changed the title Close documentation for streaming requests and shared assembly (8.6) Implement streaming requests tests and ExecPlan docs (8.6) Mar 14, 2026
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 16, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

tests/steps/streaming_request_steps.rs

Comment on file

fn then_last_stream_error_kind_is(
    streaming_request_world: &mut StreamingRequestWorld,
    kind: String,
) -> TestResult {
    let kind = match parse_error_kind(&kind) {
        Ok(kind) => kind,
        Err(err) => return Err(err.into()),
    };
    streaming_request_world.assert_last_error_kind(kind)
}

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: then_last_stream_error_kind_is,when_request_body_error_is_sent

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 16, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

tests/fixtures/streaming_request.rs

Comment on file

    pub fn assert_collected_body(&self, expected: &str) -> TestResult {
        if self.collected_body == expected.as_bytes() {
            return Ok(());
        }

        Err(format!(
            "expected body {:?}, observed {:?}",
            expected.as_bytes(),
            self.collected_body
        )
        .into())
    }

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: StreamingRequestWorld.assert_collected_body,StreamingRequestWorld.assert_collected_chunks

@coderabbitai

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

…pers

Refactored tests fixtures and steps by introducing a common helper function `assert_field_eq` to simplify equality assertions and reduce repetition in error handling assertions. Also added `try_parse_error_kind` helper in test steps to clean up error kind parsing logic, improving readability and maintainability.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos leynos changed the title Implement streaming requests tests and ExecPlan docs (8.6) Eliminate duplicated assertion code in streaming tests Mar 16, 2026
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 16, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

tests/steps/streaming_request_steps.rs

Comment on file

//! Step definitions for streaming request body scenarios.

❌ New issue: String Heavy Function Arguments
In this module, 39.1% of all arguments to its 13 functions are strings. The threshold for string arguments is 39.0%

@coderabbitai

This comment was marked as resolved.

Introduce ErrorKindArg newtype that implements FromStr to allow concise capture of io::ErrorKind from Gherkin step strings via BDD macros. This removes manual parsing and unwrap logic from the streaming_request_steps test steps, improving clarity and type safety.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
@leynos leynos changed the title Eliminate duplicated assertion code in streaming tests Add streaming inbound tests, centralize assertion helpers, and ExecPlan doc Mar 16, 2026
codescene-delta-analysis[bot]

This comment was marked as outdated.

Changed assert_field_eq function to take referenced arguments and support ?Sized trait. This makes the function more flexible and prevents unnecessary cloning. Adapted calls accordingly in streaming_request.rs test fixture.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos leynos changed the title Add streaming inbound tests, centralize assertion helpers, and ExecPlan doc Centralize streaming inbound test assertions; add ExecPlan doc Mar 17, 2026
@leynos leynos marked this pull request as ready for review March 17, 2026 10:59
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 4 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="docs/execplans/8-6-1-update-documentation-for-streaming-requests.md" line_range="14-15" />
<code_context>
+
+Roadmap items `8.6.1` through `8.6.3` close the documentation phase for
+streaming request bodies, the generic `MessageAssembler` abstraction, and the
+standardized per-connection memory-budget model introduced by
+`docs/adr-002-streaming-requests-and-shared-message-assembly.md`.
+
+After this work, a protocol author or library consumer should be able to read
</code_context>
<issue_to_address>
**issue (bug_risk):** ADR path/filename looks inconsistent with existing ADR references and may be broken.

Other docs reference this ADR as `adr/0002-streaming-requests-and-shared-message-assembly.md`, but here it’s `docs/adr-002-...` (different directory and zero-padding). Please update this reference to match the existing ADR path and filename so the link resolves correctly.
</issue_to_address>

### Comment 2
<location path="docs/execplans/8-6-1-update-documentation-for-streaming-requests.md" line_range="585" />
<code_context>
+debt, stop before marking the roadmap item done and record the exact blocking
+files in `Decision Log`.
+
+## Artifacts and notes
+
+Key evidence gathered during planning:
+
+```plaintext
+- Existing docs already contain partial 8.6 content.
+- docs/users-guide.md already documents RequestParts, MessageAssembler, and MemoryBudgets.
+- tests/streaming_request.rs exists and uses rstest.
+- No tests/features/streaming_request.feature exists today.
+```
+
+Target new artefacts if the audit confirms the current gap:
+
+```plaintext
</code_context>
<issue_to_address>
**suggestion (typo):** Mixed use of “Artifacts” and “artefacts” is inconsistent with the stated en-GB spelling guideline.

The heading and body use different spellings (“Artifacts” vs “artefacts”). Given the en-GB-oxendict guideline, please standardise on one form (likely “artefacts”) in both places.

```suggestion
## Artefacts and notes
```
</issue_to_address>

### Comment 3
<location path="docs/execplans/8-6-1-update-documentation-for-streaming-requests.md" line_range="54" />
<code_context>
+  and public API examples needed to make the documented behaviour trustworthy.
+- Do not introduce new product behaviour merely to make the documents read
+  better. If documentation work exposes an implementation bug, fix only the
+  smallest issue required to align the code with ADR 0002 and document the
+  decision.
+- Preserve existing public API signatures unless a contradiction in the current
</code_context>
<issue_to_address>
**issue (review_instructions):** ADR is used here without first expanding it (for example as "Architecture Decision Record (ADR)").

To follow the acronym rule, expand ADR the first time it appears in the document, for example "Architecture Decision Record (ADR) 0002", and then use "ADR 0002" thereafter.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.md`

**Instructions:**
Define uncommon acronyms on first use.

</details>
</issue_to_address>

### Comment 4
<location path="docs/execplans/8-6-1-update-documentation-for-streaming-requests.md" line_range="159" />
<code_context>
+- Observation: streaming request bodies currently have strong `rstest`
+  coverage in `tests/streaming_request.rs`, but there is no matching
+  behavioural suite under `tests/features/`. Impact: the implementation cannot
+  honestly claim both unit and behavioural validation for the inbound streaming
+  half of ADR 0002 until that BDD gap is closed.
+
</code_context>
<issue_to_address>
**issue (review_instructions):** BDD is referenced later as a "BDD gap" without first being defined (for example as "behaviour-driven development (BDD)").

When mentioning the "BDD gap", please expand BDD on its first occurrence in the document, such as "behaviour-driven development (BDD) gap", then use BDD on its own afterwards.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.md`

**Instructions:**
Define uncommon acronyms on first use.

</details>
</issue_to_address>

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.

Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md Outdated
Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md Outdated
Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
@coderabbitai coderabbitai Bot added the codex label Mar 17, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/execplans/8-6-1-update-documentation-for-streaming-requests.md`:
- Around line 15-16: The ADR path references use the incorrect filename
`docs/adr-002-streaming-requests-and-shared-message-assembly.md`; update all
occurrences of that string in this document to the canonical path
`docs/adr/0002-streaming-requests-and-shared-message-assembly.md` (fixing the
path at the three noted spots and any other matches) so links and audit commands
point to the ADR under docs/adr.

In `@tests/fixtures/streaming_request.rs`:
- Around line 129-136: In the async block inside
tests/fixtures/streaming_request.rs (the self.block_on(...) that uses
tokio::time::timeout(Duration::from_millis(timeout_ms), sender.send(...))),
replace the final .await.is_err() boolean collapse with an explicit match on the
timeout result so you can distinguish the three cases: return true only when the
timeout returns Err(_) (i.e., elapsed), return false when timeout yields
Ok(Ok(())) (successful send), and propagate/return an Err when timeout yields
Ok(Err(send_err)) (channel closed/send failure) so SendError is not treated as
back-pressure; use the unique symbols sender.send, tokio::time::timeout,
Duration::from_millis, and the surrounding block (the value assigned to blocked)
to locate where to implement the match.
🪄 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

Run ID: 1c7f826e-c6dc-4702-a6cb-9b32ebb279b2

📥 Commits

Reviewing files that changed from the base of the PR and between 8476be5 and 5d9fcdb.

📒 Files selected for processing (15)
  • docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
  • docs/generic-message-fragmentation-and-re-assembly-design.md
  • docs/multi-packet-and-streaming-responses-design.md
  • docs/roadmap.md
  • docs/the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md
  • docs/users-guide.md
  • src/app/builder/config.rs
  • src/app/builder/protocol.rs
  • tests/features/streaming_request.feature
  • tests/fixtures/mod.rs
  • tests/fixtures/streaming_request.rs
  • tests/scenarios/mod.rs
  • tests/scenarios/streaming_request_scenarios.rs
  • tests/steps/mod.rs
  • tests/steps/streaming_request_steps.rs

Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
Comment thread tests/fixtures/streaming_request.rs Outdated
…equests doc

Corrected typographical errors ('Artefacts') and expanded abbreviations ('ADR', 'BDD') to improve clarity and consistency in the documentation for streaming requests.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/execplans/8-6-1-update-documentation-for-streaming-requests.md`:
- Line 533: Replace the verbose phrase "all of the following are true" with the
more concise "the following are true" in the sentence that currently reads
"Acceptance means all of the following are true" so the line becomes "Acceptance
means the following are true"; update the single occurrence in the documentation
file (same sentence) to keep phrasing concise.
- Line 571: Replace the US spelling "normalization" with the British
en-GB-oxendict spelling "normalisation" in the doc where the phrase
"normalization if harmless or restore only that unrelated file before" appears;
search for the exact token "normalization" in the content of
docs/execplans/8-6-1-update-documentation-for-streaming-requests.md and update
it to "normalisation" so the documentation conforms to en-GB-oxendict spelling
rules.
- Around line 398-400: Replace the semicolon-plus-conjunction in the sentence
containing "first; and" so it reads correctly (either change the semicolon to a
comma or remove the word "and" and keep the semicolon); locate the sentence in
the document text near the phrase "a new user can follow the guide without
needing to read the ADR first; and" and update it to "a new user can follow the
guide without needing to read the ADR first, and" (or "a new user can follow the
guide without needing to read the ADR first; any Rustdoc examples...") to remove
the semicolon-before-conjunction error.
- Line 341: Update the sentence that currently reads "must configure so the two
layers do not fight each other" by inserting a comma before "so" to form "must
configure, so the two layers do not fight each other"; locate this phrase in the
document (the line containing the exact text "must configure so the two layers
do not fight each other") and apply the punctuation fix.
- Around line 179-208: The five Decision log entries in the doc (the lines that
start "- Decision: ..." and end with "Date/Author: YYYY-MM-DD / Codex") are
missing terminal periods; update each entry so the trailing "Date/Author:
YYYY-MM-DD / Codex" becomes "Date/Author: YYYY-MM-DD / Codex." for all five
occurrences (the entries listing the roadmap phase decision, the rstest-bdd
suite decision, the docs update decision, the marking-done decision, and the
streaming-request behavioural gap decision).
- Around line 1-604: Reflow the overlong bulleted list items so each
continuation line is <=80 columns (specifically the bullets that quote filenames
and long phrases in the plan's audit/observations/steps sections) by breaking
backtick-quoted filenames and long phrases across lines; also break the two
overlong bash commands ("set -o pipefail && timeout 300 make markdownlint
MDLINT=/root/.bun/bin/markdownlint-cli2 2>&1 | tee /tmp/8-6-markdownlint.log"
and "set -o pipefail && timeout 300 make doctest-benchmark 2>&1 | tee
/tmp/8-6-doctest-benchmark.log") into multiple lines using backslash
continuations inside the bash code blocks so each line <=120 columns; after
editing, run make fmt to normalise formatting.
🪄 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

Run ID: a905017d-00f0-4346-a085-78f237beb750

📥 Commits

Reviewing files that changed from the base of the PR and between 5d9fcdb and 00aac1c.

📒 Files selected for processing (1)
  • docs/execplans/8-6-1-update-documentation-for-streaming-requests.md

Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md Outdated
Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md Outdated
Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md Outdated
Comment thread docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 18, 2026

@coderabbitai Have the following now been resolved?

Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity.

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="docs/execplans/8-6-1-update-documentation-for-streaming-requests.md" line_range="14-15" />
<code_context>
+
+Roadmap items `8.6.1` through `8.6.3` close the documentation phase for
+streaming request bodies, the generic `MessageAssembler` abstraction, and the
+standardized per-connection memory-budget model introduced by
+`docs/adr-002-streaming-requests-and-shared-message-assembly.md`.
+
+After this work, a protocol author or library consumer should be able to read
</code_context>
<issue_to_address>
**issue (bug_risk):** ADR path/filename looks inconsistent with existing ADR references and may be broken.

Other docs reference this ADR as `adr/0002-streaming-requests-and-shared-message-assembly.md`, but here it’s `docs/adr-002-...` (different directory and zero-padding). Please update this reference to match the existing ADR path and filename so the link resolves correctly.
</issue_to_address>

### Comment 2
<location path="docs/execplans/8-6-1-update-documentation-for-streaming-requests.md" line_range="585" />
<code_context>
+debt, stop before marking the roadmap item done and record the exact blocking
+files in `Decision Log`.
+
+## Artifacts and notes
+
+Key evidence gathered during planning:
+
+```plaintext
+- Existing docs already contain partial 8.6 content.
+- docs/users-guide.md already documents RequestParts, MessageAssembler, and MemoryBudgets.
+- tests/streaming_request.rs exists and uses rstest.
+- No tests/features/streaming_request.feature exists today.
+```
+
+Target new artefacts if the audit confirms the current gap:
+
+```plaintext
</code_context>
<issue_to_address>
**suggestion (typo):** Mixed use of “Artifacts” and “artefacts” is inconsistent with the stated en-GB spelling guideline.

The heading and body use different spellings (“Artifacts” vs “artefacts”). Given the en-GB-oxendict guideline, please standardise on one form (likely “artefacts”) in both places.

```suggestion
## Artefacts and notes
```
</issue_to_address>

### Comment 3
<location path="docs/execplans/8-6-1-update-documentation-for-streaming-requests.md" line_range="54" />
<code_context>
+  and public API examples needed to make the documented behaviour trustworthy.
+- Do not introduce new product behaviour merely to make the documents read
+  better. If documentation work exposes an implementation bug, fix only the
+  smallest issue required to align the code with ADR 0002 and document the
+  decision.
+- Preserve existing public API signatures unless a contradiction in the current
</code_context>
<issue_to_address>
**issue (review_instructions):** ADR is used here without first expanding it (for example as "Architecture Decision Record (ADR)").

To follow the acronym rule, expand ADR the first time it appears in the document, for example "Architecture Decision Record (ADR) 0002", and then use "ADR 0002" thereafter.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.md`

**Instructions:**
Define uncommon acronyms on first use.

</details>
</issue_to_address>

### Comment 4
<location path="docs/execplans/8-6-1-update-documentation-for-streaming-requests.md" line_range="159" />
<code_context>
+- Observation: streaming request bodies currently have strong `rstest`
+  coverage in `tests/streaming_request.rs`, but there is no matching
+  behavioural suite under `tests/features/`. Impact: the implementation cannot
+  honestly claim both unit and behavioural validation for the inbound streaming
+  half of ADR 0002 until that BDD gap is closed.
+
</code_context>
<issue_to_address>
**issue (review_instructions):** BDD is referenced later as a "BDD gap" without first being defined (for example as "behaviour-driven development (BDD)").

When mentioning the "BDD gap", please expand BDD on its first occurrence in the document, such as "behaviour-driven development (BDD) gap", then use BDD on its own afterwards.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.md`

**Instructions:**
Define uncommon acronyms on first use.

</details>
</issue_to_address>

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 18, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd '8-6-1-update-documentation-for-streaming-requests.md' --exec cat {}

Length of output: 28277


@leynos: Three of the four comments are resolved. Here is the full status:

# Issue Status
1 ADR path inconsistency (docs/adr-002-… vs adr/0002-…) Not resolved
2 Mixed "Artifacts" / "artefacts" heading ✅ Resolved (## Artefacts and notes)
3 ADR acronym not expanded on first use ✅ Resolved ("Architecture Decision Record (ADR) 0002")
4 BDD acronym not expanded on first use ✅ Resolved ("behaviour-driven development (BDD) gap")

No requirements should be annulled — all four are consistent with en-GB-oxendict spelling and none increases cyclomatic complexity.


Comment 1 remains open. The file still uses docs/adr-002-streaming-requests-and-shared-message-assembly.md in at least three places (Purpose/big-picture, Constraints, and Context sections). Correct those to adr/0002-streaming-requests-and-shared-message-assembly.md to match the existing cross-references elsewhere in the repository.

Diff:

-`docs/adr-002-streaming-requests-and-shared-message-assembly.md`.
+`adr/0002-streaming-requests-and-shared-message-assembly.md`.

Apply the same substitution to every remaining occurrence in the file.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 18, 2026

@coderabbitai the file is named docs/adr-002-streaming-requests-and-shared-message-assembly.md

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 18, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the actual ADR filename and location
fd 'adr' --type f

Length of output: 279


@leynos: Confirmed — the file lives at docs/adr-002-streaming-requests-and-shared-message-assembly.md, which is exactly what the ExecPlan uses. My Comment 1 was incorrect; the reference in the document is right as-is. Apologies for the noise.

Updated status:

# Issue Status
1 ADR path inconsistency ✅ No issue — path is correct
2 Mixed "Artifacts" / "artefacts" heading ✅ Resolved
3 ADR acronym not expanded on first use ✅ Resolved
4 BDD acronym not expanded on first use ✅ Resolved

All four comments are resolved.

… backpressure states accurately

- Improved backpressure detection logic in `send_chunk_with_timeout` by differentiating the timeout, successful send, and channel error cases.
- Enhanced handling ensures clearer indication of whether send is blocked due to backpressure or channel error.
- Minor editorial fixes in documentation for clarity and style without changing meaning.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (2)
docs/execplans/8-6-1-update-documentation-for-streaming-requests.md (2)

513-519: ⚠️ Potential issue | 🟡 Minor

Split overlong gate commands in the bash block.

Fix Line 513 and Line 519 to keep each code-block line at or below 120
columns.

🔧 Proposed fix
-   set -o pipefail && timeout 300 make markdownlint MDLINT=/root/.bun/bin/markdownlint-cli2 2>&1 | tee /tmp/8-6-markdownlint.log
+   set -o pipefail && timeout 300 make markdownlint \
+     MDLINT=/root/.bun/bin/markdownlint-cli2 2>&1 \
+     | tee /tmp/8-6-markdownlint.log
@@
-   set -o pipefail && timeout 300 make doctest-benchmark 2>&1 | tee /tmp/8-6-doctest-benchmark.log
+   set -o pipefail && timeout 300 make doctest-benchmark 2>&1 \
+     | tee /tmp/8-6-doctest-benchmark.log

As per coding guidelines, “Code blocks in Markdown must be wrapped at 120
columns.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/execplans/8-6-1-update-documentation-for-streaming-requests.md` around
lines 513 - 519, Split the two overlong bash lines so no code-block line exceeds
120 columns: for the line starting with "set -o pipefail && timeout 300 make
markdownlint MDLINT=/root/.bun/bin/markdownlint-cli2 2>&1 | tee
/tmp/8-6-markdownlint.log" break after "make markdownlint" and move the
MDLINT=... and the pipe to tee onto subsequent indented lines using backslashes;
similarly for the line starting with "set -o pipefail && timeout 300 make
doctest-benchmark 2>&1 | tee /tmp/8-6-doctest-benchmark.log" break before the
"|" and place the pipe on the next indented line with a trailing backslash to
keep each physical line <= 120 chars.

570-572: ⚠️ Potential issue | 🟡 Minor

Use en-GB spelling: normalisation.

Replace normalization at Line 571 with normalisation.

🇬🇧 Proposed fix
-3. if `make fmt` rewrote an unrelated document, either keep the clean
-   normalization if harmless or restore only that unrelated file before
+3. if `make fmt` rewrote an unrelated document, either keep the clean
+   normalisation if harmless or restore only that unrelated file before
    continuing.

As per coding guidelines, “Documentation must use en-GB-oxendict spelling and
grammar.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/execplans/8-6-1-update-documentation-for-streaming-requests.md` around
lines 570 - 572, Replace the American spelling "normalization" with en-GB
"normalisation" in the sentence that begins "if `make fmt` rewrote an unrelated
document..." (the occurrence currently at Line 571) so the document uses
en-GB-oxendict spelling; update only that word and leave surrounding punctuation
and spacing unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/fixtures/streaming_request.rs`:
- Around line 185-198: In drain_stream, drop the sender before you start
draining: set self.sender = None immediately at the start of pub fn
drain_stream(&mut self) (before taking self.stream) so the channel won’t stay
open; this change touches drain_stream and ensures the subsequent block_on async
loop that reads from stream (using classify_next_chunk and stream.next()) can
observe EOF rather than blocking.
- Around line 101-103: The create_channel method currently forwards
user-supplied capacity to body_channel which will panic if capacity == 0; change
create_channel to return Result<(), String>, validate that capacity > 0 and
return Err("request body channel capacity must be greater than zero") when it is
zero, otherwise call body_channel(capacity), assign self.sender and self.stream,
clear self.collected_body, reset self.collected_chunks, self.last_error_kind and
self.send_blocked_by_backpressure as before, and return Ok(()) on success so
callers can handle the error instead of allowing a panic.

---

Duplicate comments:
In `@docs/execplans/8-6-1-update-documentation-for-streaming-requests.md`:
- Around line 513-519: Split the two overlong bash lines so no code-block line
exceeds 120 columns: for the line starting with "set -o pipefail && timeout 300
make markdownlint MDLINT=/root/.bun/bin/markdownlint-cli2 2>&1 | tee
/tmp/8-6-markdownlint.log" break after "make markdownlint" and move the
MDLINT=... and the pipe to tee onto subsequent indented lines using backslashes;
similarly for the line starting with "set -o pipefail && timeout 300 make
doctest-benchmark 2>&1 | tee /tmp/8-6-doctest-benchmark.log" break before the
"|" and place the pipe on the next indented line with a trailing backslash to
keep each physical line <= 120 chars.
- Around line 570-572: Replace the American spelling "normalization" with en-GB
"normalisation" in the sentence that begins "if `make fmt` rewrote an unrelated
document..." (the occurrence currently at Line 571) so the document uses
en-GB-oxendict spelling; update only that word and leave surrounding punctuation
and spacing 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

Run ID: 9704852a-64ee-4d0e-b12f-3bb41c9fc616

📥 Commits

Reviewing files that changed from the base of the PR and between 00aac1c and f866234.

📒 Files selected for processing (2)
  • docs/execplans/8-6-1-update-documentation-for-streaming-requests.md
  • tests/fixtures/streaming_request.rs

Comment thread tests/fixtures/streaming_request.rs Outdated
Comment thread tests/fixtures/streaming_request.rs
… is zero

Added error handling to StreamingRequestWorld::create_channel to return an error when capacity is zero, preventing invalid channel creation. Updated related test step to propagate the error accordingly.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
@leynos leynos changed the title Centralize streaming inbound test assertions; add ExecPlan doc Centralize streaming inbound assertions; add ExecPlan doc and tests Mar 18, 2026
@leynos leynos merged commit b24d1c7 into main Mar 18, 2026
6 checks passed
@leynos leynos deleted the update-docs-streaming-requests-yqxepp branch March 18, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant