Conversation
Expand the user guide with custom FrameCodec guidance and example\nreferences.\n\nAdd codec testing tasks to the roadmap, update ADR-004 with\nimplementation guidance, and introduce a test observability ADR\nfor log and metrics capture.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughSummarise additions: add FrameCodec implementation guidance and test recommendations; introduce ADR‑006 defining a per‑test ObservabilityHandle; make wireframe_testing codec‑aware with new drive and encode/decode helpers; add user docs for creating and installing custom frame codecs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Test code
participant Obs as ObservabilityHandle
participant Driver as Wireframe test driver
participant Logger as Scoped Logger
participant Metrics as Metrics recorder
rect rgba(230,245,255,0.9)
Note over Test,Obs: Initialise per-test observability
Test->>Obs: ObservabilityHandle::new()
Obs--)Logger: install scoped log capture
Obs--)Metrics: register in-process recorder
end
rect rgba(245,255,230,0.9)
Note over Test,Driver: Exercise codec paths
Test->>Driver: drive_with_frames / drive_with_payloads (codec)
Driver->>Logger: emit logs
Driver->>Metrics: record metrics
Driver-->>Test: return framed responses
end
rect rgba(255,245,230,0.9)
Note over Test,Obs: Inspect and tear down
Test->>Obs: snapshot() / logs() / counter(...)
Obs-->>Test: return captured data
Test->>Obs: drop
Obs--)Logger: restore prior logger state
Obs--)Metrics: restore prior recorder
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)docs/**/*.md📄 CodeRabbit inference engine (docs/contents.md)
Files:
docs/**/*.{rs,md}📄 CodeRabbit inference engine (docs/rust-doctest-dry-guide.md)
Files:
**/*.md📄 CodeRabbit inference engine (AGENTS.md)
Files:
⚙️ CodeRabbit configuration file
Files:
🔍 Remote MCP DeepwikiRelevant facts to support reviewing PR #414 (docs + wireframe_testing changes):
Actionable checks for the reviewer (match these against PR text/changes):
Sources used: repository documentation and wireframe_testing pages via Deepwiki_read_wiki_contents. ⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
|
Reviewer's GuideExpands documentation around pluggable FrameCodec usage, introduces implementation guidance and testing roadmap items for codec-aware test harnesses, and adds a new ADR defining a test observability harness for logs and metrics capture in tests. Sequence diagram for codec-aware driver with test observabilitysequenceDiagram
actor Test
participant Obs as ObservabilityHandle
participant Log as LoggerHandle
participant Metrics as DebuggingRecorder
participant Driver as WireframeTestingHelpers
participant App as WireframeApp
participant Codec as FrameCodec
Test->>Obs: new()
Obs->>Log: install_log_capture()
Obs->>Metrics: install_metrics_recorder()
Test->>Obs: clear()
Obs->>Log: clear()
Obs->>Metrics: snapshot_and_discard()
Test->>Driver: drive_with_payloads(app, codec, payloads)
activate Driver
Driver->>Codec: wrap_payload(payloads)
Codec-->>Driver: frames
Driver->>App: handle_connection(duplex_stream)
activate App
App->>Codec: encoder(), decoder()
App->>Metrics: record(codec_bytes_in)
App->>Metrics: record(codec_bytes_out)
App->>Log: log(codec_error or success)
App-->>Driver: response_bytes
deactivate App
Driver->>Codec: decode(response_bytes)
Codec-->>Driver: response_frames
Driver-->>Test: response_frames
deactivate Driver
Test->>Obs: snapshot()
Obs->>Metrics: snapshot()
Metrics-->>Obs: counters
Obs-->>Test: Snapshot
Test->>Obs: counter("wireframe.codec.errors", [])
Obs->>Metrics: scan_counters()
Metrics-->>Obs: value
Obs-->>Test: u64
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
Align the testing crate design with pluggable FrameCodec support and the observability harness from ADR 006. Document codec-aware driver entry points, fixture expectations, and log/metrics capture behaviour so implementers can plan the work.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- Several of the documented APIs in
wireframe_testing(e.g.,drive_with_raw_bytes’s signature missing a codec parameter,FrameCodec::wrap_payload/frame_payloadhelpers,TestSerializerbounds, and theobservability()/ObservabilityHandlesurface) are only sketched here; when you implement them, please align names and signatures exactly with the docs or adjust the docs accordingly to avoid drift and confusion for users treating this as authoritative. - The observability harness is described as serialising access with a global lock and requiring tests not to run concurrently; consider being more explicit in the ADR and user-facing docs about how this interacts with popular Rust test runners’ parallel execution (e.g., recommending
--test-threads=1or a per-suite fixture pattern) so users don’t accidentally write flaky observability tests.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several of the documented APIs in `wireframe_testing` (e.g., `drive_with_raw_bytes`’s signature missing a codec parameter, `FrameCodec::wrap_payload`/`frame_payload` helpers, `TestSerializer` bounds, and the `observability()`/`ObservabilityHandle` surface) are only sketched here; when you implement them, please align names and signatures exactly with the docs or adjust the docs accordingly to avoid drift and confusion for users treating this as authoritative.
- The observability harness is described as serialising access with a global lock and requiring tests not to run concurrently; consider being more explicit in the ADR and user-facing docs about how this interacts with popular Rust test runners’ parallel execution (e.g., recommending `--test-threads=1` or a per-suite fixture pattern) so users don’t accidentally write flaky observability tests.
## Individual Comments
### Comment 1
<location> `docs/wireframe-testing-crate.md:109-118` </location>
<code_context>
+ E: Packet,
+ F: FrameCodec;
+
+pub async fn drive_with_raw_bytes<S, C, E, F>(
+ app: WireframeApp<S, C, E, F>,
+ wire_bytes: Vec<Vec<u8>>,
+ capacity: Option<usize>,
+) -> io::Result<Vec<u8>>
+where
+ S: TestSerializer,
+ C: Send + 'static,
+ E: Packet,
+ F: FrameCodec;
```
</code_context>
<issue_to_address>
**suggestion:** Align `drive_with_raw_bytes` signature with the surrounding description of codec-aware drivers.
The earlier description says primary driver APIs take both the app and its codec, but `drive_with_raw_bytes` doesn’t take a `codec: &F` argument even though it’s generic over `F: FrameCodec`. Please either add the codec parameter or remove/adjust the `F: FrameCodec` bound and update the surrounding text so the description matches the function signature.
```suggestion
pub async fn drive_with_raw_bytes<S, C, E, F>(
app: WireframeApp<S, C, E, F>,
codec: &F,
wire_bytes: Vec<Vec<u8>>,
capacity: Option<usize>,
) -> io::Result<Vec<u8>>
where
S: TestSerializer,
C: Send + 'static,
E: Packet,
F: FrameCodec;
```
</issue_to_address>
### Comment 2
<location> `docs/users-guide.md:131` </location>
<code_context>
+- Define a `Frame` type and paired decoder/encoder implementations that return
+ `std::io::Error` on failure.
+- Return only the logical payload bytes from `frame_payload` so metadata parsing
+ and deserialization run against the right buffer.
+- Wrap outbound payloads with `wrap_payload`, adding any protocol headers or
+ metadata required by the wire format.
</code_context>
<issue_to_address>
**issue (review_instructions):** Use en-GB spelling here: "deserialization" should be "deserialisation".
To follow the en-GB-oxendic spelling guideline, update "deserialization" to "deserialisation" on this line.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Use en-GB-oxendic (-ize / -yse / -our) spelling and grammar.
</details>
</issue_to_address>
### Comment 3
<location> `docs/users-guide.md:142` </location>
<code_context>
+
+Install a custom codec with `with_codec`. The builder resets fragmentation to
+the codec-derived defaults, so override fragmentation afterwards if the
+protocol uses a different budget. When you already have a framed stream, use
+`send_response_framed_with_codec` so responses pass through
+`FrameCodec::wrap_payload`.
</code_context>
<issue_to_address>
**issue (review_instructions):** This sentence uses the 2nd person pronoun "you", which the style guide prohibits.
The phrase "When you already have a framed stream" violates the no 1st/2nd person pronouns rule. Consider rephrasing, for example: "When a framed stream is already available, use ..." or similar neutral wording.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Avoid 2nd person or 1st person pronouns ("I", "you", "we").
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
docs/adr-004-pluggable-protocol-codecs.mddocs/adr-006-test-observability.mddocs/roadmap.mddocs/users-guide.mddocs/wireframe-testing-crate.md
🧰 Additional context used
📓 Path-based instructions (8)
docs/**/*.md
📄 CodeRabbit inference engine (docs/contents.md)
Follow the documentation style guide conventions when writing project documentation
Write official documentation with separate guides for 'Duplex Messaging & Pushes', 'Streaming Responses', and 'Message Fragmentation', each including runnable examples and explaining relevant concepts and APIs
docs/**/*.md: Use markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in thedocs/directory to reflect the latest state. Ensure documentation remains accurate and current.
Documentation must use en-GB-oxendict spelling and grammar ('-ize' / '-yse' / '-our'), with the exception of the filenameLICENSE.
Reference the documentation style guide provided atdocs/documentation-style-guide.mdfor documentation writing standards.Documentation describing multi-packet streaming MUST reflect the tuple-based API and explain helper constructors that prepare the channel pair
Replace or retire documentation that refers to Cucumber, including guides and examples in docs/
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-GB-oxendict), including: -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse), -our suffixes (colour, behaviour), -re suffixes (calibre, centre), double 'l' (cancelled, counsellor), maintain 'e' (likeable, liveable), -ogue suffixes (analogue, catalogue)
Keep US spelling when used in an API, for example 'color'
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation: 'Lille Industries are expanding'
Write headings in sentence case
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Follow markdownlint recommendations for markdown forma...
Files:
docs/users-guide.mddocs/adr-004-pluggable-protocol-codecs.mddocs/adr-006-test-observability.mddocs/roadmap.mddocs/wireframe-testing-crate.md
docs/**/*.{rs,md}
📄 CodeRabbit inference engine (docs/rust-doctest-dry-guide.md)
Every doctest should validate the public API of a crate from the perspective of an external user, treating each documentation test as a separate temporary crate that imports the library as an external dependency
Files:
docs/users-guide.mddocs/adr-004-pluggable-protocol-codecs.mddocs/adr-006-test-observability.mddocs/roadmap.mddocs/wireframe-testing-crate.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes in Markdown.
Files:
docs/users-guide.mddocs/adr-004-pluggable-protocol-codecs.mddocs/adr-006-test-observability.mddocs/roadmap.mddocs/wireframe-testing-crate.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/users-guide.mddocs/adr-004-pluggable-protocol-codecs.mddocs/adr-006-test-observability.mddocs/roadmap.mddocs/wireframe-testing-crate.md
docs/users-guide.md
📄 CodeRabbit inference engine (docs/roadmap.md)
Extend
docs/users-guide.mdwith client configuration tables, lifecycle diagrams, and troubleshooting guidance
Files:
docs/users-guide.md
docs/adr-[0-9][0-9][0-9]-*.md
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
Name ADR (Architectural Decision Record) files using the pattern 'adr-NNN-short-description.md', where NNN is a zero-padded sequence number, and place ADRs in the docs/ directory
Files:
docs/adr-004-pluggable-protocol-codecs.mddocs/adr-006-test-observability.md
docs/adr-*.md
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
docs/adr-*.md: Every ADR must include the following sections in order: Status (one of Proposed, Accepted, Superseded, or Deprecated), Date (YYYY-MM-DD format), and Context and Problem Statement
In ADRs, include Decision Drivers, Requirements, Options Considered, Decision Outcome / Proposed Direction, Goals and Non-Goals, Migration Plan, Known Risks and Limitations, Outstanding Decisions, and Architectural Rationale sections as appropriate to the decision's complexity
In ADRs, use second-level headings (##) for major sections and third-level headings (###) for subsections
In ADRs, include tables when comparing options across multiple dimensions and add a caption below the table (e.g. 'Table 1: Trade-offs between X and Y.')
In ADRs, include code snippets with language identifiers when illustrating technical approaches, and use 'no_run' for illustrative Rust code that should not be executed
In ADRs, add screen reader descriptions before complex diagrams or code blocks
In ADRs, reference external sources using inline links or footnotes
Files:
docs/adr-004-pluggable-protocol-codecs.mddocs/adr-006-test-observability.md
docs/adr-004-pluggable-protocol-codecs.md
📄 CodeRabbit inference engine (docs/roadmap.md)
Update
docs/adr-004-pluggable-protocol-codecs.mdwith FrameCodec trait changes
Files:
docs/adr-004-pluggable-protocol-codecs.md
docs/**/roadmap*.md
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
docs/**/roadmap*.md: In roadmap task writing, define outcomes, not intentions: phrase tasks in terms of the capability delivered (e.g. 'Implement role-based access control for API endpoints'), not aspirations like 'Improve security'
In roadmap task writing, quantify completion criteria and attach measurable finish lines (e.g. '90% test coverage for new modules', 'response times under 200ms', 'all endpoints migrated')
In roadmap task writing, break into atomic increments: ensure tasks can be completed in weeks, not quarters, and decompose large goals into clear, deliverable units
In roadmap task writing, tie to dependencies and sequencing: document prerequisites so tasks can be scheduled realistically
In roadmap task writing, bound scope explicitly: note both in-scope and out-of-scope elements
In roadmap documentation, use three layers of scope: Phases (strategic milestones, numbered 1, 2, 3), Steps (epics/workstreams, numbered 1.1, 1.2, 1.3), and Tasks (execution units, numbered 1.1.1, 1.1.2, 1.1.3)
In roadmap documentation, use dotted numbering for phases, steps, and headline tasks, and precede task and sub-task items with a GitHub-Flavoured Markdown (GFM) checkbox '[ ]' to track completion status
In roadmap documentation, note non-linear dependencies explicitly using dotted notation (e.g. 'Requires 2.3.1')
In roadmap documentation, include explicit success criteria only where not immediately obvious from the task description
In roadmap documentation, cite relevant design document sections using dotted notation (e.g. 'See design-doc.md §3.2')
Files:
docs/roadmap.md
🧠 Learnings (2)
📚 Learning: 2026-01-02T04:41:11.015Z
Learnt from: CR
Repo: leynos/wireframe PR: 0
File: docs/roadmap.md:0-0
Timestamp: 2026-01-02T04:41:11.015Z
Learning: Applies to docs/adr-004-pluggable-protocol-codecs.md : Update `docs/adr-004-pluggable-protocol-codecs.md` with FrameCodec trait changes
Applied to files:
docs/adr-004-pluggable-protocol-codecs.md
📚 Learning: 2026-01-02T04:39:36.933Z
Learnt from: CR
Repo: leynos/wireframe PR: 0
File: docs/adr-004-pluggable-protocol-codecs.md:0-0
Timestamp: 2026-01-02T04:39:36.933Z
Learning: Tie buffer sizing and maximum frame length configuration to codec implementation to keep transport-level constraints close to the framing rules that define them
Applied to files:
docs/adr-004-pluggable-protocol-codecs.md
🪛 LanguageTool
docs/adr-006-test-observability.md
[style] ~18-~18: To elevate your writing, try using a synonym here.
Context: ...y assertions inconsistent, brittle, and hard to reuse across downstream crates. A d...
(HARD_TO)
[uncategorized] ~21-~21: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ted test observability harness is needed so tests can assert on logging and metrics...
(COMMA_COMPOUND_SENTENCE_2)
[style] ~79-~79: Would you like to use the Oxford spelling “Serialize”? The spelling ‘Serialise’ is also correct.
Context: ... by labels when verifying counters. - Serialise access with a global lock to avoid cros...
(OXFORD_SPELLING_Z_NOT_S)
[style] ~86-~86: Would you like to use the Oxford spelling “serialization”? The spelling ‘serialisation’ is also correct.
Context: ...etup. - Global recorder access requires serialisation, which may reduce parallelism for obs...
(OXFORD_SPELLING_Z_NOT_S)
docs/wireframe-testing-crate.md
[uncategorized] ~14-~14: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ses a unified test observability harness so tests can assert on logs and metrics de...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~134-~134: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...are all returned as io::Error values so tests can assert on error handling. ##...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~204-~204: Possible missing comma found.
Context: ...`. - Access is serialized with a global lock so concurrent tests do not interfere. -...
(AI_HYDRA_LEO_MISSING_COMMA)
🔍 Remote MCP Deepwiki
Summary of Additional Context for PR #414 Review
Based on my research of the wireframe repository, here are the key facts relevant to reviewing this pull request:
Project Architecture Context
The wireframe framework uses a FrameProcessor trait to abstract frame parsing and encoding logic, with a built-in LengthPrefixedProcessor supporting configurable length-prefixed framing. The WireframeApp is a generic builder struct with three type parameters: S for Serializer, C for Connection State, and E for Packet (message envelope).
Existing Testing Infrastructure
The wireframe_testing crate provides utilities for testing WireframeApp instances by simulating network connections using in-memory streams, with core components including drive_with_* functions, LoggerHandle for thread-safe log capture, and rstest integration for fixtures. The drive_internal function sets up duplex streams, runs the server function, writes frames to the client side, and collects output while handling panics in the server task.
Current Observability Capabilities
The wireframe framework provides comprehensive observability through the tracing crate for structured logging with hierarchical span structures (connection, request, response_stream, push spans), and the metrics crate for operational metrics including connections_active, frames_processed_total, push_queue_depth, and request_latency_seconds.
Codec and Protocol Customization
Wireframe provides several extension points for protocol customization: a public FrameProcessor trait for custom framing implementations, a Serializer trait for pluggable serialization backends with default bincode integration, and a Packet trait for custom envelope types.
Relevance to PR Changes
The PR documentation changes align with the existing architecture: ADR-004 adds implementation guidance for codec-specific tests that leverage the existing FrameProcessor trait flexibility; ADR-006 proposes a test observability harness that complements the existing LoggerHandle and extends it with in-process metrics capture (addressing a current gap in test-time metrics validation); and the roadmap tasks establish codec-aware testing patterns that build upon the existing wireframe_testing helper functions and rstest fixtures.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: Sourcery review
🔇 Additional comments (5)
docs/adr-004-pluggable-protocol-codecs.md (1)
270-270: Footnote format is correct.The cross-reference to ADR-006 using GitHub-flavoured Markdown footnote syntax is well integrated. Retain as-is after rewrapping the bullet points above.
docs/roadmap.md (2)
584-585: Footnote reference format is correct.The cross-reference to ADR-006 using GitHub-flavoured Markdown footnote syntax is well placed. The footnote definition is correctly formatted. Retain as-is after rewrapping the task items above.
428-438: Section 9.7 already complies with the 80-column wrapping requirement. All lines measure within the boundary (maximum 79 characters), so no rewrapping is necessary.Likely an incorrect or invalid review comment.
docs/adr-006-test-observability.md (1)
1-110: ADR structure and decision rationale are sound.The ADR follows best practices: Status and Date are clear, Context and Problem Statement articulate the observability gap, Decision Drivers are well-motivated, and Options Considered provide defensible trade-offs. The Roadmap section (Phase 1 and Phase 2) is a valuable addition for phasing implementation. Technical approach (ObservabilityHandle with logtest and metrics-util, global serialisation lock, drop-based restoration) is well-reasoned and aligns with existing testing infrastructure.
docs/wireframe-testing-crate.md (1)
1-288: Documentation structure is comprehensive and technically sound.The rewritten crate documentation provides clear motivation, design goals, and crate layout. Codec-aware driver entry points (
drive_with_frames,drive_with_payloads,drive_with_raw_bytes, and mutable variants) are well-documented with behavioural details. The buffer capacity guidance and frame encoding/decoding helpers are appropriately scoped. The observability harness section accurately reflects the ADR-006 proposal (ObservabilityHandle, DebuggingRecorder, per-test isolation, drop restoration). Fixtures and example usage are concrete and helpful. Implementation notes bridge documentation to roadmap tasks.Cross-references to ADR-004 and ADR-006 strengthen alignment with decision records.
Refine testing crate documentation for codec-aware drivers and observability guidance, and align the test observability ADR. Update the users guide framing note to remove second-person phrasing and clarify framed-stream usage.
This comment was marked as resolved.
This comment was marked as resolved.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2747 @leynos: I've checked the current state of the files. Here's the status: Resolved ✓Comment 1 (drive_with_raw_bytes signature): Resolved by removing the Comment 3 (pronoun usage): Resolved. Line 142 now reads "When a framed stream is already available," removing the second-person pronoun "you". Overall observability concurrency guidance: Resolved. ADR-006 now includes explicit guidance (lines 81-83) recommending Not Resolved ✗Comment 2 (en-GB spelling): Line 131 of Change this line: - Return only the logical payload bytes from `frame_payload` so metadata parsing
and deserialization run against the right buffer.To: - Return only the logical payload bytes from `frame_payload` so metadata parsing
and deserialisation run against the right buffer. |
Update the custom codec guidance to use en-GB spelling for "deserialisation".
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2915 @leynos: Yes, all the review comments have now been resolved! Here's the status: ✓ All ResolvedComment 1 (drive_with_raw_bytes signature): The Comment 2 (en-GB spelling): Line 133 of Comment 3 (pronoun usage): Line 144 of Overall comment (concurrency guidance): ADR-006 now includes explicit guidance (lines 81-83) recommending |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
docs/adr-006-test-observability.mddocs/users-guide.mddocs/wireframe-testing-crate.md
🧰 Additional context used
📓 Path-based instructions (6)
docs/**/*.md
📄 CodeRabbit inference engine (docs/contents.md)
Follow the documentation style guide conventions when writing project documentation
Write official documentation with separate guides for 'Duplex Messaging & Pushes', 'Streaming Responses', and 'Message Fragmentation', each including runnable examples and explaining relevant concepts and APIs
docs/**/*.md: Use markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in thedocs/directory to reflect the latest state. Ensure documentation remains accurate and current.
Documentation must use en-GB-oxendict spelling and grammar ('-ize' / '-yse' / '-our'), with the exception of the filenameLICENSE.
Reference the documentation style guide provided atdocs/documentation-style-guide.mdfor documentation writing standards.Documentation describing multi-packet streaming MUST reflect the tuple-based API and explain helper constructors that prepare the channel pair
Replace or retire documentation that refers to Cucumber, including guides and examples in docs/
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-GB-oxendict), including: -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse), -our suffixes (colour, behaviour), -re suffixes (calibre, centre), double 'l' (cancelled, counsellor), maintain 'e' (likeable, liveable), -ogue suffixes (analogue, catalogue)
Keep US spelling when used in an API, for example 'color'
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation: 'Lille Industries are expanding'
Write headings in sentence case
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Follow markdownlint recommendations for markdown forma...
Files:
docs/adr-006-test-observability.mddocs/wireframe-testing-crate.mddocs/users-guide.md
docs/**/*.{rs,md}
📄 CodeRabbit inference engine (docs/rust-doctest-dry-guide.md)
Every doctest should validate the public API of a crate from the perspective of an external user, treating each documentation test as a separate temporary crate that imports the library as an external dependency
Files:
docs/adr-006-test-observability.mddocs/wireframe-testing-crate.mddocs/users-guide.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes in Markdown.
Files:
docs/adr-006-test-observability.mddocs/wireframe-testing-crate.mddocs/users-guide.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/adr-006-test-observability.mddocs/wireframe-testing-crate.mddocs/users-guide.md
docs/adr-[0-9][0-9][0-9]-*.md
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
Name ADR (Architectural Decision Record) files using the pattern 'adr-NNN-short-description.md', where NNN is a zero-padded sequence number, and place ADRs in the docs/ directory
Files:
docs/adr-006-test-observability.md
docs/adr-*.md
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
docs/adr-*.md: Every ADR must include the following sections in order: Status (one of Proposed, Accepted, Superseded, or Deprecated), Date (YYYY-MM-DD format), and Context and Problem Statement
In ADRs, include Decision Drivers, Requirements, Options Considered, Decision Outcome / Proposed Direction, Goals and Non-Goals, Migration Plan, Known Risks and Limitations, Outstanding Decisions, and Architectural Rationale sections as appropriate to the decision's complexity
In ADRs, use second-level headings (##) for major sections and third-level headings (###) for subsections
In ADRs, include tables when comparing options across multiple dimensions and add a caption below the table (e.g. 'Table 1: Trade-offs between X and Y.')
In ADRs, include code snippets with language identifiers when illustrating technical approaches, and use 'no_run' for illustrative Rust code that should not be executed
In ADRs, add screen reader descriptions before complex diagrams or code blocks
In ADRs, reference external sources using inline links or footnotes
Files:
docs/adr-006-test-observability.md
docs/users-guide.md
📄 CodeRabbit inference engine (docs/roadmap.md)
Extend
docs/users-guide.mdwith client configuration tables, lifecycle diagrams, and troubleshooting guidance
Files:
docs/users-guide.md
🪛 LanguageTool
docs/adr-006-test-observability.md
[style] ~18-~18: To elevate your writing, try using a synonym here.
Context: ...y assertions inconsistent, brittle, and hard to reuse across downstream crates. A d...
(HARD_TO)
docs/users-guide.md
[style] ~131-~131: Would you like to use the Oxford spelling “deserialization”? The spelling ‘deserialisation’ is also correct.
Context: ...rame_payload` so metadata parsing and deserialisation run against the right buffer. - Wrap ou...
(OXFORD_SPELLING_Z_NOT_S)
🔍 Remote MCP Deepwiki
Summary — additional facts relevant to reviewing PR #414
-
wireframe_testing crate: provides codec-aware test helpers and fixtures used by the docs changes — key APIs include drive_with_frame(s), drive_with_bincode, run_app/run_with_duplex_server, TestSerializer trait bounds, processor() and logger() rstest fixtures, and LoggerHandle for test log capture. These are implemented with tokio::io::duplex, configurable buffer capacities, and panic isolation via catch_unwind.,
-
Observability / ADR-006 specifics referenced in the PR: design proposes wireframe_testing::observability with an ObservabilityHandle that installs scoped log capture (logtest) and an in-process metrics recorder, provides snapshot/inspection helpers, serializes access with a global lock to avoid cross-test interference, and recommends running tests serially or sharing fixtures (e.g., --test-threads=1) to avoid flaky assertions.
-
Frame/codec testing guidance aligns with existing framing and serializer traits: Wireframe supports FrameProcessor/LengthPrefixedProcessor, Serializer (BincodeSerializer) and FrameMetadata; tests should use existing helpers (drive_with_*) and TestSerializer bounds to exercise encoder/decoder round-trips, payload extraction, correlation IDs, and max frame length behavior. The testing utilities already provide capacity controls and bincode integration (drive_with_bincode).
-
drive_with_raw_bytes / drive_with_* API surface: the testing docs show several drive_with_* overloads (single/multiple frames, capacity/mutable variants). If docs mention a codec-bound on a helper (e.g., drive_with_raw_bytes with F: FrameCodec), confirm the docs match the implemented helpers (many drive helpers are codec-agnostic; codec-aware variants encode/decode via TestSerializer).
References (tools used)
- Repo documentation and crate docs: Deepwiki_read_wiki_contents (leynos/wireframe) — pages covering Testing, wireframe_testing crate, Observability, Frame/Serializer, and related API surfaces.
- Wiki structure overview listing Testing and wireframe_testing topics.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (3)
docs/wireframe-testing-crate.md (2)
219-236: This is a proposed API specification—implement the documented interface as written.The types referenced in the proposed API do exist in the codebase:
LoggerHandleis fully implemented inwireframe_testing/src/logging.rsmetrics_util::debugging::Snapshotis available (version 0.20.0 in dev dependencies) and already used intests/metrics.rsThe documented signatures align with existing testing infrastructure and ADR-006. Implement
ObservabilityHandlewith the specified public API:
counter(name: &str, labels: &[(&str, &str)]) -> u64for querying metrics with label filteringsnapshot() -> Snapshotfor capturing the current recorder statelogs() -> &mut LoggerHandlefor accessing log captureEnsure the implementation correctly integrates
LoggerHandleacquisition and metrics recorder setup within the global lock constraint documented at line 203–206.
111-119: Removedrive_with_raw_bytesfrom documentation or implement the function.The function
drive_with_raw_bytesdocumented at lines 111–119 does not exist in thewireframe_testingcrate. The actual public API exports onlydrive_with_frame,drive_with_frames,drive_with_bincode, and their variants (_mut,_with_capacity). These functions acceptWireframeApp<S, C, E>(three generics) rather than the four-generic variant shown in the documentation.Either implement
drive_with_raw_bytesinwireframe_testing/src/helpers.rsand export it fromlib.rs, or remove the description and signature from lines 78–119 in the documentation.Likely an incorrect or invalid review comment.
docs/users-guide.md (1)
128-138: No changes needed. Verification confirms the description at lines 132–133 is accurate:wrap_payloadis correctly described as a method for wrapping outbound payloads with protocol headers or metadata. The implementation uses it exclusively on the send path (viasend_response_payload,drive_with_payloads, and similar), confirming it is outbound-only and not bidirectional.
Expand the testing crate design with mutable driver signatures and a metrics example that aligns with existing counters. Clarify the test observability ADR consequences with concrete parallelism guidance.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docs/adr-006-test-observability.md (1)
85-92: Cross-reference the Mitigation and rollback section explicitly in Consequences.The reference to "the mitigation guidance in the approach" is vague. Past review feedback requested explicit cross-reference to lines 108–113 (the Mitigation and rollback section) to ensure readers are aware of fallback strategies (feature flag, log-based assertions) available if the global recorder approach becomes problematic. Clarify that the Approach section covers the primary strategy (--test-threads=1, shared fixtures), and the Mitigation and rollback section (lines 109–114) provides additional fallback options.
Proposed fix
- Global recorder access requires serialization, which may reduce parallelism - for observability-heavy suites, such as forcing `--test-threads=1` for the - affected test binary. See the mitigation guidance in the approach. + Global recorder access requires serialisation, which may reduce parallelism + for observability-heavy suites, such as forcing `--test-threads=1` for the + affected test binary. See the Approach section for primary strategies + and the Mitigation and rollback section (lines 109–114) for fallback options + including feature flags and log-based assertions.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
docs/adr-006-test-observability.mddocs/wireframe-testing-crate.md
🧰 Additional context used
📓 Path-based instructions (5)
docs/**/*.md
📄 CodeRabbit inference engine (docs/contents.md)
Follow the documentation style guide conventions when writing project documentation
Write official documentation with separate guides for 'Duplex Messaging & Pushes', 'Streaming Responses', and 'Message Fragmentation', each including runnable examples and explaining relevant concepts and APIs
docs/**/*.md: Use markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in thedocs/directory to reflect the latest state. Ensure documentation remains accurate and current.
Documentation must use en-GB-oxendict spelling and grammar ('-ize' / '-yse' / '-our'), with the exception of the filenameLICENSE.
Reference the documentation style guide provided atdocs/documentation-style-guide.mdfor documentation writing standards.Documentation describing multi-packet streaming MUST reflect the tuple-based API and explain helper constructors that prepare the channel pair
Replace or retire documentation that refers to Cucumber, including guides and examples in docs/
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-GB-oxendict), including: -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse), -our suffixes (colour, behaviour), -re suffixes (calibre, centre), double 'l' (cancelled, counsellor), maintain 'e' (likeable, liveable), -ogue suffixes (analogue, catalogue)
Keep US spelling when used in an API, for example 'color'
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation: 'Lille Industries are expanding'
Write headings in sentence case
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Follow markdownlint recommendations for markdown forma...
Files:
docs/adr-006-test-observability.mddocs/wireframe-testing-crate.md
docs/**/*.{rs,md}
📄 CodeRabbit inference engine (docs/rust-doctest-dry-guide.md)
Every doctest should validate the public API of a crate from the perspective of an external user, treating each documentation test as a separate temporary crate that imports the library as an external dependency
Files:
docs/adr-006-test-observability.mddocs/wireframe-testing-crate.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes in Markdown.
Files:
docs/adr-006-test-observability.mddocs/wireframe-testing-crate.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/adr-006-test-observability.mddocs/wireframe-testing-crate.md
docs/adr-[0-9][0-9][0-9]-*.md
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
Name ADR (Architectural Decision Record) files using the pattern 'adr-NNN-short-description.md', where NNN is a zero-padded sequence number, and place ADRs in the docs/ directory
Files:
docs/adr-006-test-observability.md
docs/adr-*.md
📄 CodeRabbit inference engine (docs/documentation-style-guide.md)
docs/adr-*.md: Every ADR must include the following sections in order: Status (one of Proposed, Accepted, Superseded, or Deprecated), Date (YYYY-MM-DD format), and Context and Problem Statement
In ADRs, include Decision Drivers, Requirements, Options Considered, Decision Outcome / Proposed Direction, Goals and Non-Goals, Migration Plan, Known Risks and Limitations, Outstanding Decisions, and Architectural Rationale sections as appropriate to the decision's complexity
In ADRs, use second-level headings (##) for major sections and third-level headings (###) for subsections
In ADRs, include tables when comparing options across multiple dimensions and add a caption below the table (e.g. 'Table 1: Trade-offs between X and Y.')
In ADRs, include code snippets with language identifiers when illustrating technical approaches, and use 'no_run' for illustrative Rust code that should not be executed
In ADRs, add screen reader descriptions before complex diagrams or code blocks
In ADRs, reference external sources using inline links or footnotes
Files:
docs/adr-006-test-observability.md
🪛 LanguageTool
docs/adr-006-test-observability.md
[style] ~18-~18: To elevate your writing, try using a synonym here.
Context: ...y assertions inconsistent, brittle, and hard to reuse across downstream crates. A d...
(HARD_TO)
🔍 Remote MCP Deepwiki
Relevant facts gathered for reviewing PR #414
- wireframe_testing crate already exposes codec-aware test helpers and the public driver functions described in the PR (drive_with_frame(s), drive_with_payloads, drive_with_raw_bytes, mutable variants, encode/decode helpers) and documents the TestSerializer trait bounds; helpers live in wireframe_testing/src/helpers.rs.
- ObservabilityHandle and observability() concepts (scoped log capture, per-test metrics recorder, global serialization/lock, snapshot/clear/counter helpers) are documented as part of the testing crate design and testing utilities (wireframe_testing/src/logging.rs).
- The testing helpers use tokio::io::duplex, support configurable buffer capacities (DEFAULT_CAPACITY = 4096, MAX_CAPACITY ≈ 10MB), and implement panic isolation (catch_unwind) — relevant when ADR-006 recommends serialisation or running tests with --test-threads=1.
- wireframe core docs show FrameProcessor/LengthPrefixedProcessor, Serializer/BincodeSerializer, FrameMetadata, and correlation_id propagation are already first-class concepts — confirms the PR's codec guidance (frame_payload/wrap_payload, max_frame_length, correlation IDs) aligns with existing architecture.
- The roadmap and testing pages already call out adding codec fixtures, codec-aware drivers, and a test observability harness (the PR mirrors those roadmap items).
Sources used:,
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: build-test
🔇 Additional comments (2)
docs/adr-006-test-observability.md (1)
79-79: Fix British English spelling for -ise verbs.Change "Serialize access" to "Serialise access" to align with en-GB-oxendict spelling rules.
Proposed fix
- Serialize access with a global lock to avoid cross-test interference, and + Serialise access with a global lock to avoid cross-test interference, andLikely an incorrect or invalid review comment.
docs/wireframe-testing-crate.md (1)
149-150: No action required. The documentation correctly usesF::wrap_payloadandF::frame_payloadas trait methods onFrameCodec. Both are defined insrc/codec.rsas static methods with signatures matching their documented usage. The examples at lines 149–150 and 193–202 accurately represent the API.
Introduce payload-driven helpers in wireframe_testing and re-export them for tests. Update the testing crate design doc to reflect the current helper signatures, correct the bincode wrapper example, and expand the observability concurrency guidance.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
docs/wireframe-testing-crate.mdwireframe_testing/src/helpers.rswireframe_testing/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Runmake check-fmt(cargo fmt --workspace -- --check) before committing to validate Rust formatting across the entire workspace without modifying files.
Runmake lint(cargo clippy --workspace --all-targets --all-features -- -D warnings) before committing to lint every target with all features enabled and deny all Clippy warnings.
Runmake test(cargo test --workspace) before committing to run the full workspace test suite.
Clippy warnings MUST be disallowed. Fix any warnings emitted during tests in the code itself rather than silencing them.
Usemake fmt(cargo fmt --workspace) to apply formatting fixes reported by the formatter check.
Where a Rust function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a Rust function has too many parameters, group related parameters in meaningfully named structs.
Where a Rust function is returning a large error, consider usingArcto reduce the amount of data returned.
Write unit and behavioural tests for new Rust functionality. Run both before and after making any change.
Every Rust module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public Rust APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessarymutbindings in Rust code.
Avoidunsafecode in Rust unless absolutely necessary, and document any usage clearly with a 'SAFETY' comment.
Place function attributes after doc comments in Rust code.
Do not usereturnin single-line Rust functions.
Use predicate functions for Rust conditional criteria with more than two branches.
Lints must not be silenced in Rust except as a last resort. Lint rule suppressions must be tightly scoped and include a clear reason.
Useconcat!()to combine long string literals in Rust rather than escaping newlines with a backslash.
Prefer single-line versions of Rust functions where...
Files:
wireframe_testing/src/lib.rswireframe_testing/src/helpers.rs
⚙️ CodeRabbit configuration file
**/*.rs: * Seek to keep the cognitive complexity of functions no more than 9.
- Adhere to single responsibility and CQRS
- Place function attributes after doc comments.
- Do not use
returnin single-line functions.- Move conditionals with >2 branches into a predicate function.
- Avoid
unsafeunless absolutely necessary.- Every module must begin with a
//!doc comment that explains the module's purpose and utility.- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.- Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
- Use
rstestfixtures for shared setup and to avoid repetition between tests.- Replace duplicated tests with
#[rstest(...)]parameterised cases.- Prefer
mockallfor mocks/stubs.- Prefer
.expect()over.unwrap()in tests..expect()and.unwrap()are forbidden outside of tests. Errors must be propagated.- Ensure that any API or behavioural changes are reflected in the documentation in
docs/- Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/- Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
- Environment access (env::set_var and env::remove_var) are always unsafe in Rust 2024 and MUST be marked as such
- For testing of functionality depending upon environment variables, dependency injection and...
Files:
wireframe_testing/src/lib.rswireframe_testing/src/helpers.rs
docs/**/*.md
📄 CodeRabbit inference engine (docs/contents.md)
Follow the documentation style guide conventions when writing project documentation
Write official documentation with separate guides for 'Duplex Messaging & Pushes', 'Streaming Responses', and 'Message Fragmentation', each including runnable examples and explaining relevant concepts and APIs
docs/**/*.md: Use markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in thedocs/directory to reflect the latest state. Ensure documentation remains accurate and current.
Documentation must use en-GB-oxendict spelling and grammar ('-ize' / '-yse' / '-our'), with the exception of the filenameLICENSE.
Reference the documentation style guide provided atdocs/documentation-style-guide.mdfor documentation writing standards.Documentation describing multi-packet streaming MUST reflect the tuple-based API and explain helper constructors that prepare the channel pair
Replace or retire documentation that refers to Cucumber, including guides and examples in docs/
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-GB-oxendict), including: -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse), -our suffixes (colour, behaviour), -re suffixes (calibre, centre), double 'l' (cancelled, counsellor), maintain 'e' (likeable, liveable), -ogue suffixes (analogue, catalogue)
Keep US spelling when used in an API, for example 'color'
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation: 'Lille Industries are expanding'
Write headings in sentence case
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Follow markdownlint recommendations for markdown forma...
Files:
docs/wireframe-testing-crate.md
docs/**/*.{rs,md}
📄 CodeRabbit inference engine (docs/rust-doctest-dry-guide.md)
Every doctest should validate the public API of a crate from the perspective of an external user, treating each documentation test as a separate temporary crate that imports the library as an external dependency
Files:
docs/wireframe-testing-crate.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes in Markdown.
Files:
docs/wireframe-testing-crate.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/wireframe-testing-crate.md
🧬 Code graph analysis (2)
wireframe_testing/src/lib.rs (1)
wireframe_testing/src/helpers.rs (2)
drive_with_payloads(297-307)drive_with_payloads_mut(321-331)
wireframe_testing/src/helpers.rs (2)
src/app/envelope.rs (3)
payload(162-162)new(85-91)new(117-123)src/frame/format.rs (4)
default(112-112)err(142-143)err(145-145)new(32-35)
🪛 LanguageTool
docs/wireframe-testing-crate.md
[uncategorized] ~124-~124: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ould be added as non-breaking extensions so tests can pass FrameCodec values and ...
(COMMA_COMPOUND_SENTENCE_2)
[style] ~231-~231: Would you like to use the Oxford spelling “amortize”? The spelling ‘amortise’ is also correct.
Context: ...ilityHandle` via a per-suite fixture to amortise setup costs. - When partial parallelism...
(OXFORD_SPELLING_Z_NOT_S)
🔍 Remote MCP Deepwiki
Summary of additional repository facts relevant to reviewing PR #414
-
Documentation sources (docs/ and wireframe_testing crate) already describe and implement codec-aware testing helpers, per-test observability, and the ObservabilityHandle/observability() concepts; these docs and test helpers live under wireframe_testing and related docs.,
-
wireframe_testing exposes drive helpers and re-exports:
- drive_with_payloads / drive_with_payloads_mut implemented in wireframe_testing/src/helpers.rs and re-exported from wireframe_testing::helpers.
-
Test harness behavior and constraints:
- duplex-based in-memory testing uses tokio::io::duplex with DEFAULT_CAPACITY = 4096 and MAX_CAPACITY ≈ 10MB (configurable capacity variants). Panic isolation uses AssertUnwindSafe + catch_unwind so server task panics surface as io::Error in tests. These details affect ADR-006 guidance about serialisation and running tests with --test-threads=1.
-
Observability/test harness facts:
- ObservabilityHandle and observability() are documented as the per-test API surface (methods like new(), logs(), snapshot(), clear(), counter()). Docs emphasize global serialization (global lock) to avoid cross-test interference and recommend serial test execution or --test-threads=1. Verify PR doc names/signatures match actual wireframe_testing API to avoid doc/impl drift.
-
FrameCodec and codec testing alignment:
- Core types (FrameProcessor, Serializer, FrameMetadata, Packet/Envelope, correlation_id propagation) are implemented in core and testing docs; FrameCodec-related helper names (frame_payload, wrap_payload, max_frame_length, TestSerializer trait bounds) must match these existing abstractions or be updated in docs.
-
Roadmap and ADR linking:
- Roadmap.md and ADR-004 already reference adding codec fixtures and a test observability harness; ADR-006 (new) should be consistent with these roadmap entries.
Actionable checks for the PR reviewer
- Confirm documented function/type names and signatures (drive_with_raw_bytes, drive_with_payloads, ObservabilityHandle/observability, FrameCodec helper names, TestSerializer bounds) exactly match implementations in wireframe_testing/src/helpers.rs and wireframe_testing/src/logging.rs.
- Verify guidance in ADR-006 about global lock/serial tests includes explicit instruction (and examples) for running Rust tests with --test-threads=1 or using per-suite fixtures; confirm tests/docs show the DEFAULT_CAPACITY and MAX_CAPACITY values and panic-isolation behavior.
- Ensure roadmap cross-reference footnote to ADR-006 is present and accurate.
Sources
- Repo documentation and testing crate contents retrieved via Deepwiki_read_wiki_contents and Deepwiki_read_wiki_structure.,
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: build-test
🔇 Additional comments (6)
wireframe_testing/src/helpers.rs (3)
282-307: LGTM!The
drive_with_payloadsfunction provides a clean abstraction for tests that need length-prefixed framing without manually constructing frames. The delegation to the capacity variant follows the established pattern in this module.
333-351: LGTM!The
encode_payloadshelper encapsulates the framing logic cleanly, returning anio::Resultto propagate encoding failures. The pre-allocation usingLengthFormat::default().bytes()is consistent withencode_frameat line 523.
604-624: LGTM!The test validates the round-trip behaviour of
drive_with_payloadsby encoding anEnvelope, driving it through the app, and verifying the payload survives the journey intact. The pattern match onframes.as_slice()is idiomatic for asserting exactly one response frame.wireframe_testing/src/lib.rs (1)
37-38: LGTM!The new payload driver helpers are correctly re-exported, extending the crate's public API surface for tests that need automatic length-prefixed framing.
docs/wireframe-testing-crate.md (2)
1-35: LGTM!The introduction and crate layout provide a clear overview of the testing crate's purpose and structure. The design goals align well with ADR 004 and ADR 006.
272-305: LGTM!The example tests demonstrate realistic usage of the driver and observability helpers. The
round_trips_with_codecexample correctly usesdrive_with_bincodeanddecode_frames, andcaptures_metricsshows the observability harness API.
Adjust punctuation and clarify which codec helpers are proposed, plus correct the bincode example assertions and spelling.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/wireframe-testing-crate.md
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.md
📄 CodeRabbit inference engine (docs/contents.md)
Follow the documentation style guide conventions when writing project documentation
Write official documentation with separate guides for 'Duplex Messaging & Pushes', 'Streaming Responses', and 'Message Fragmentation', each including runnable examples and explaining relevant concepts and APIs
docs/**/*.md: Use markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in thedocs/directory to reflect the latest state. Ensure documentation remains accurate and current.
Documentation must use en-GB-oxendict spelling and grammar ('-ize' / '-yse' / '-our'), with the exception of the filenameLICENSE.
Reference the documentation style guide provided atdocs/documentation-style-guide.mdfor documentation writing standards.Documentation describing multi-packet streaming MUST reflect the tuple-based API and explain helper constructors that prepare the channel pair
Replace or retire documentation that refers to Cucumber, including guides and examples in docs/
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-GB-oxendict), including: -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse), -our suffixes (colour, behaviour), -re suffixes (calibre, centre), double 'l' (cancelled, counsellor), maintain 'e' (likeable, liveable), -ogue suffixes (analogue, catalogue)
Keep US spelling when used in an API, for example 'color'
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation: 'Lille Industries are expanding'
Write headings in sentence case
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Follow markdownlint recommendations for markdown forma...
Files:
docs/wireframe-testing-crate.md
docs/**/*.{rs,md}
📄 CodeRabbit inference engine (docs/rust-doctest-dry-guide.md)
Every doctest should validate the public API of a crate from the perspective of an external user, treating each documentation test as a separate temporary crate that imports the library as an external dependency
Files:
docs/wireframe-testing-crate.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes in Markdown.
Files:
docs/wireframe-testing-crate.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/wireframe-testing-crate.md
🔍 Remote MCP Deepwiki
Relevant facts to help review PR #414 (documentation + wireframe_testing docs / helpers):
-
New/changed testing APIs documented and implemented in wireframe_testing:
- drive_with_payloads / drive_with_payloads_mut added and re-exported; they encode payloads as length-delimited frames and return response bytes (helpers in wireframe_testing/src/helpers.rs).
- ObservabilityHandle API documented with: new(), logs(&mut), snapshot(&self), clear(&mut), counter(&self, name: &str, labels: &[(&str,&str)]) -> u64, plus observability() -> ObservabilityHandle entry point.
- encode_frames / decode_frames and codec-aware drive helpers documented for codec-aware testing (docs/wireframe-testing-crate).
-
Test harness / observability behavior and constraints:
- Test observability ADR (ADR-006) proposes an ObservabilityHandle that installs scoped log capture and a metrics recorder, uses a global lock/serialization to avoid cross-test interference, and recommends running tests serially or with --test-threads=1 (or per-suite fixtures) to prevent interference. ADR-006 describes phased rollout: capture primitives first, then wire codec/recovery assertions.
- wireframe_testing uses tokio::io::duplex with DEFAULT_CAPACITY = 4096 and MAX_CAPACITY ≈ 10MB for in-memory test buffers; panic isolation in test drives uses AssertUnwindSafe + catch_unwind and converts server-task panics into io::Error for tests. These are relevant to ADR-006's guidance on serialization/parallelism.
-
Codec testing guidance / docs alignment points called out by reviewer:
- Users/docs now describe codec-aware helpers and FrameCodec bounds; reviewers should confirm doc signatures (drive_with_raw_bytes, drive_with_payloads, ObservabilityHandle/observability, FrameCodec helper names like frame_payload/wrap_payload, and TestSerializer trait bounds) exactly match implementations in wireframe_testing/src/helpers.rs and related modules.
Actionable checks for reviewer (derived from repo docs):
- Verify documented function/type names and signatures match implementations:
- drive_with_payloads / drive_with_payloads_mut implementations and re-exports in wireframe_testing.
- ObservabilityHandle and observability() API surface in docs vs wireframe_testing logging/observability implementation.
- Any FrameCodec/TestSerializer helper names in users-guide and ADR-004 match code (helpers and trait bounds).
- Confirm ADR-006 includes explicit guidance for running tests serially (--test-threads=1) and documents global lock/serialization behavior and the DEFAULT_CAPACITY / MAX_CAPACITY / panic-isolation details so users can follow mitigations in CI.
Sources consulted: repository docs and wireframe_testing docs/helpers via Deepwiki_read_wiki_contents and structure.
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: build-test
🔇 Additional comments (1)
docs/wireframe-testing-crate.md (1)
293-305: Verify thatinc_frames()in the globalmetricsmodule updates the scoped recorder inObservabilityHandle.The example calls
inc_frames(Direction::Inbound)as a global function at line 299 and then asserts thatobs.counter(...)captures the incremented value at lines 300–303. This assumes that the observability handle's scoped metrics recorder is active wheninc_frames()is called.Confirm that:
- The observability harness installs a scoped
metricsrecorder before returning the handle- Global
metrics::function calls (likeinc_frames) route to the active scoped recorder during the handle's lifetime- The example correctly reflects this behaviour
Verify in wireframe_testing/src/observability.rs that the ObservabilityHandle::new() method (or observability() function) installs a scoped metrics recorder and that global metrics functions respect this scoping.
Clarify the testing crate documentation by splitting current frame helper signatures from proposed codec-aware additions. Update the bincode example assertion and punctuation to avoid confusion about available APIs.
1 participant
Expand the user guide with custom FrameCodec guidance and example\nreferences.\n\nAdd codec testing tasks to the roadmap, update ADR-004 with\nimplementation guidance, and introduce a test observability ADR\nfor log and metrics capture.
Summary by Sourcery
Expand documentation for codec-aware testing, custom FrameCodec usage, and introduce a test observability ADR.
Documentation: