Wireframe v0.3.0: scoped task-local context and fallible app factory#299
Merged
Wireframe v0.3.0: scoped task-local context and fallible app factory#299
Conversation
Contributor
OverviewThis pull request completes migration of mxd to wireframe v0.3.0. It replaces per-connection thread-local state with a scoped Tokio task-local context model, introduces a fallible app-factory pattern that surfaces app-construction errors (fail-closed), updates integration points to carry context through wiring rather than TLS, tightens server bootstrap validation, and refreshes Cargo.lock alongside the wireframe dependency bump 0.2.0 → 0.3.0. Notable correctness safeguard: validate the protocol tag length in the handshake (enforce exactly 4 bytes) before sending a Hotline handshake to avoid invalid-tag runtime errors. Core implementation changes
Tests
Documentation
Reviewer feedback & remediationAll reviewer suggestions were applied:
Files of interest (high-review-effort)
Related design docs / traceability
WalkthroughUpgrade Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant WFServer as WireframeServer
participant Handshake as HandshakeHook
participant ScopeCtx as scope_current_context
participant Registry as Registry
participant AppFactory as AppFactory
Client->>WFServer: TCP connect + send handshake bytes
WFServer->>Handshake: deliver handshake bytes
Handshake->>ScopeCtx: build ConnectionContext
ScopeCtx->>Registry: install context (task-local + registry)
ScopeCtx->>AppFactory: invoke factory to build app
AppFactory-->>ScopeCtx: return Result<HotlineApp, AppFactoryError>
ScopeCtx-->>WFServer: handshake complete (ok or error)
WFServer-->>Client: send handshake reply code
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
Reviewer's GuideMigrates the project to wireframe v0.3.0 by updating the dependency, fixing breaking API changes, documenting new runtime/testing/client capabilities, tightening server app-factory failure handling, and expanding the docs with a migration execplan, repository layout, and extensive wireframe user guide additions (codecs, fragmentation, memory budgets, streaming, testing, and client features). Sequence diagram for WireframeClient connect, preamble, hooks, and correlated callsequenceDiagram
actor User
participant Builder as WireframeClientBuilder
participant Client as WireframeClient
participant Stream as TcpStream
participant Hooks as ClientHooks
participant Server as WireframeServer
User->>Builder: serializer(...), codec_config(...), socket_options(...)
User->>Builder: with_preamble(preamble)
User->>Builder: on_connection_setup(...), on_connection_teardown(...)
User->>Builder: on_error(...)
User->>Builder: before_send(...), after_receive(...)
User->>Builder: connect(addr)
Builder->>Stream: tcp_connect(addr)
Builder->>Stream: write_preamble(preamble)
Builder->>Stream: read_preamble_response()
alt preamble_success
Builder->>Hooks: on_connection_setup()
Hooks-->>Builder: state
Builder-->>User: Client
else preamble_failure
Builder->>Hooks: on_error(ClientError::PreambleRead or ClientError::PreambleTimeout or ClientError::PreambleEncode)
Builder-->>User: error
end
rect rgb(235,235,255)
User->>Client: call_correlated(request_envelope)
Client->>Client: next_correlation_id()
Client->>Client: set_correlation_id_on_envelope()
Client->>Hooks: before_send(serialized_bytes)
Client->>Stream: write_frame(bytes)
Server-->>Stream: read_frame()
Server-->>Client: response_frame
Client->>Hooks: after_receive(raw_frame_bytes)
Client->>Client: validate_correlation_id()
alt ids_match
Client-->>User: response_envelope
else ids_mismatch
Client->>Hooks: on_error(ClientError::CorrelationMismatch)
Client-->>User: error
end
end
User->>Client: close()
Client->>Hooks: on_connection_teardown(state)
Client->>Stream: shutdown()
Client-->>User: ()
Class diagram for unified error surface, app factory, and memory budgetsclassDiagram
class WireframeError~E~ {
+DuplicateRoute(u32)
+Io(std_io_Error)
+Protocol(E)
+Codec(CodecError)
}
class CodecError {
}
class Result~T,E~ {
<<type_alias>>
}
class WireframeApp~Ser,Ctx,Env,Codec~ {
+memory_budgets: MemoryBudgets
+fragmentation: Option~FragmentationConfig~
+codec: Codec
+new() Result~WireframeApp,WireframeError~
+memory_budgets(budgets: MemoryBudgets) WireframeApp
+enable_fragmentation() WireframeApp
+fragmentation(config: Option~FragmentationConfig~) WireframeApp
+with_message_assembler(assembler: impl MessageAssembler) WireframeApp
+message_assembler() Option~Arc_MessageAssembler~~
}
class FragmentationConfig {
+max_message_size: usize
+reassembly_timeout: Duration
}
class MemoryBudgets {
+bytes_per_message: BudgetBytes
+bytes_per_connection: BudgetBytes
+bytes_in_flight: BudgetBytes
+new(per_message: BudgetBytes, per_connection: BudgetBytes, in_flight: BudgetBytes) MemoryBudgets
+derived_from_frame_budget(frame_budget: usize) MemoryBudgets
}
class BudgetBytes {
+value: NonZeroUsize
+new(value: NonZeroUsize) BudgetBytes
+get() usize
}
class MessageAssembler {
<<trait>>
+parse_frame_header(payload: &[u8]) Result~ParsedFrameHeader,io_Error~
}
class ParsedFrameHeader {
+header: FrameHeader
+header_len: usize
+new(header: FrameHeader, header_len: usize) ParsedFrameHeader
}
class FrameHeader {
<<enum>>
First
Continuation
}
class FirstFrameHeader {
+message_key: MessageKey
+metadata_len: usize
+body_len: usize
+total_body_len: Option~usize~
+is_last: bool
}
class ContinuationFrameHeader {
+message_key: MessageKey
+sequence: Option~FrameSequence~
+body_len: usize
+is_last: bool
}
class MessageKey {
+value: u64
}
class FrameSequence {
+value: u64
}
class MessageAssemblyState {
+max_message_size: NonZeroUsize
+timeout: Duration
+accept_first_frame(input: FirstFrameInput) Result~Option~AssembledMessage~,MessageAssemblyError~
+accept_continuation_frame(header: ContinuationFrameHeader, body: &[u8]) Result~Option~AssembledMessage~,MessageAssemblyError~
}
class FirstFrameInput {
+new(header: &FirstFrameHeader, routing: EnvelopeRouting, metadata: Vec~u8~, body: &[u8]) Result~FirstFrameInput,MessageAssemblyError~
}
class AssembledMessage {
+routing: EnvelopeRouting
+metadata: Vec~u8~
+body: Vec~u8~
}
class EnvelopeRouting {
+envelope_id: EnvelopeId
+correlation_id: Option~CorrelationId~
}
class EnvelopeId {
+value: u32
}
class CorrelationId {
+value: u64
}
class MessageSeries {
+from_first_frame(header: &FirstFrameHeader) MessageSeries
+accept_continuation(header: &ContinuationFrameHeader) Result~MessageSeriesStatus,MessageSeriesError~
}
class MessageSeriesStatus {
<<enum>>
Incomplete
Complete
}
class MessageSeriesError {
<<enum>>
SequenceMismatch
DuplicateFrame
SeriesComplete
}
class AppFactory~Ser,Ctx,E,Codec~ {
<<trait>>
+call() R
}
class FactoryResult~App~ {
<<trait>>
}
class WireframeServer~F,Pre,S,Ser,Ctx,E,Codec~ {
+new(factory: F) WireframeServer
+bind(addr: SocketAddr) Result~WireframeServer,WireframeError~
+run_with_shutdown(signal: ShutdownSignal) Result~(),WireframeError~
}
class HotlineApp {
<<type_alias>>
}
class AppFactoryError {
<<enum>>
MissingHandshakeContext
MissingPeerAddress
BuildApplication(String)
}
WireframeError --> CodecError
Result --> WireframeError
WireframeApp --> MemoryBudgets
WireframeApp --> FragmentationConfig
WireframeApp ..> MessageAssembler
MemoryBudgets --> BudgetBytes
ParsedFrameHeader --> FrameHeader
FrameHeader --> FirstFrameHeader
FrameHeader --> ContinuationFrameHeader
FirstFrameHeader --> MessageKey
ContinuationFrameHeader --> MessageKey
ContinuationFrameHeader --> FrameSequence
MessageAssemblyState --> FirstFrameInput
MessageAssemblyState --> AssembledMessage
FirstFrameInput --> FirstFrameHeader
FirstFrameInput --> EnvelopeRouting
AssembledMessage --> EnvelopeRouting
EnvelopeRouting --> EnvelopeId
EnvelopeRouting --> CorrelationId
MessageSeries --> FirstFrameHeader
MessageSeries --> ContinuationFrameHeader
MessageSeries --> MessageSeriesStatus
MessageSeries --> MessageSeriesError
AppFactory ..|> Fn
AppFactory ..> FactoryResult
WireframeServer --> AppFactory
FactoryResult <|.. WireframeApp
FactoryResult <|.. Result
HotlineApp <|-- WireframeApp
AppFactoryError --> WireframeError
Class diagram for Wireframe client streaming, pooling, hooks, and tracingclassDiagram
class WireframeClientBuilder {
+serializer(serializer: Ser) WireframeClientBuilder
+codec_config(config: ClientCodecConfig) WireframeClientBuilder
+socket_options(options: SocketOptions) WireframeClientBuilder
+with_preamble(preamble: T) WireframeClientBuilder
+preamble_timeout(timeout: Duration) WireframeClientBuilder
+on_connection_setup(f: FnOnceSetup) WireframeClientBuilder
+on_connection_teardown(f: FnOnceTeardown) WireframeClientBuilder
+on_error(f: FnErrorHook) WireframeClientBuilder
+before_send(f: BeforeSendHook) WireframeClientBuilder
+after_receive(f: AfterReceiveHook) WireframeClientBuilder
+tracing_config(config: TracingConfig) WireframeClientBuilder
+connect(addr: SocketAddr) Result~WireframeClient,ClientError~
+connect_pool(addr: SocketAddr, config: ClientPoolConfig) Result~WireframeClientPool,ClientError~
}
class WireframeClient {
+call~Req,Res~(request: Req) Result~Res,ClientError~
+send~Req~(request: Req) Result~(),ClientError~
+receive~Res~() Result~Res,ClientError~
+send_envelope~P~(envelope: P) Result~u64,ClientError~
+receive_envelope~P~() Result~P,ClientError~
+call_correlated~P~(request: P) Result~P,ClientError~
+call_streaming~P~(request: P) Result~ResponseStream~P~,ClientError~
+receive_streaming~P~(correlation_id: u64) ResponseStream~P~
+send_streaming(header: &[u8], body: impl AsyncRead, config: SendStreamingConfig) Result~SendStreamingOutcome,ClientError~
+close() Future~Output = ()~
+next_correlation_id() u64
}
class ClientError {
<<enum>>
Io(std_io_Error)
Wireframe(WireframeError)
Serialize(String)
PreambleEncode(String)
PreambleRead(String)
PreambleTimeout
CorrelationMismatch
StreamCorrelationMismatch
}
class ResponseStream~P~ {
<<futures_Stream>>
-client: &mut WireframeClient
-correlation_id: u64
+poll_next() Poll~Option~Result~P,ClientError~~~
}
class StreamingResponseExt~S,P~ {
<<trait>>
+typed_with~Item,Mapper~(mapper: Mapper) TypedResponseStream~S,Mapper,P,Item~
}
class TypedResponseStream~S,Mapper,P,Item~ {
<<futures_Stream>>
-inner: S
-mapper: Mapper
+poll_next() Poll~Option~Result~Item,ClientError~~~
}
class SendStreamingConfig {
+chunk_size: Option~usize~
+timeout: Option~Duration~
+with_chunk_size(size: usize) SendStreamingConfig
+with_timeout(timeout: Duration) SendStreamingConfig
}
class SendStreamingOutcome {
+frames_sent() usize
+bytes_sent() usize
}
class ClientCodecConfig {
+max_frame_length: usize
+new(max_frame_length: usize) ClientCodecConfig
}
class SocketOptions {
+nodelay: Option~bool~
+send_buffer_size: Option~usize~
+recv_buffer_size: Option~usize~
}
class TracingConfig {
+with_all_levels(level: Level) TracingConfig
+with_all_timing(enabled: bool) TracingConfig
+with_connect_level(level: Level) TracingConfig
+with_call_level(level: Level) TracingConfig
+with_streaming_level(level: Level) TracingConfig
+with_connect_timing(enabled: bool) TracingConfig
+with_call_timing(enabled: bool) TracingConfig
}
class BeforeSendHook {
<<type_alias Fn_mut_Vec_u8_ref_mut_>>
}
class AfterReceiveHook {
<<type_alias Fn_mut_BytesMut_ref_mut_>>
}
class FnOnceSetup {
<<closure>>
+call() Future~Output = State~
}
class FnOnceTeardown {
<<closure>>
+call(state: State) Future~Output = ()~
}
class FnErrorHook {
<<closure>>
+call(error: ClientError) Future~Output = ()~
}
class WireframeClientPool {
+handle() PoolHandle
+acquire() Future~Output = Result~PooledClientLease,ClientError~~
+close() Future~Output = ()~
}
class ClientPoolConfig {
+pool_size: usize
+max_in_flight_per_socket: usize
+idle_timeout: Duration
+fairness_policy: PoolFairnessPolicy
+pool_size(size: usize) ClientPoolConfig
+max_in_flight_per_socket(limit: usize) ClientPoolConfig
+idle_timeout(timeout: Duration) ClientPoolConfig
+fairness_policy(policy: PoolFairnessPolicy) ClientPoolConfig
}
class PoolFairnessPolicy {
<<enum>>
RoundRobin
Lifo
}
class PoolHandle {
+acquire() Future~Output = Result~PooledClientLease,ClientError~~
}
class PooledClientLease {
<<Deref_Target = WireframeClient>>
-client: WireframeClient
+call~Req,Res~(request: Req) Result~Res,ClientError~
+send~Req~(request: Req) Result~(),ClientError~
+receive~Res~() Result~Res,ClientError~
}
class Envelope {
+id: u32
+correlation_id: Option~u64~
+payload: Vec~u8~
+new(id: u32, correlation_id: Option~u64~, payload: Vec~u8~) Envelope
+payload_bytes() &[u8]
}
class Packet {
<<trait>>
+id(&self) u32
+into_parts(self) PacketParts
+from_parts(parts: PacketParts) Self
+is_stream_terminator(&self) bool
}
class PacketParts {
+id() u32
+correlation_id() Option~u64~
+into_payload(self) Vec~u8~
+new(id: u32, correlation_id: Option~u64~, payload: Vec~u8~) PacketParts
}
class CorrelatableFrame {
<<trait>>
+correlation_id(&self) Option~u64~
+set_correlation_id(&mut self, id: Option~u64~)
}
WireframeClientBuilder --> WireframeClient
WireframeClientBuilder --> WireframeClientPool
WireframeClientBuilder --> ClientCodecConfig
WireframeClientBuilder --> SocketOptions
WireframeClientBuilder --> TracingConfig
WireframeClientBuilder --> BeforeSendHook
WireframeClientBuilder --> AfterReceiveHook
WireframeClientBuilder --> FnOnceSetup
WireframeClientBuilder --> FnOnceTeardown
WireframeClientBuilder --> FnErrorHook
WireframeClient --> ResponseStream
WireframeClient --> SendStreamingConfig
WireframeClient --> SendStreamingOutcome
WireframeClient --> ClientError
ResponseStream --> WireframeClient
StreamingResponseExt <|.. ResponseStream
StreamingResponseExt --> TypedResponseStream
TypedResponseStream --> ResponseStream
SendStreamingConfig --> SendStreamingOutcome
WireframeClientPool --> ClientPoolConfig
WireframeClientPool --> PoolHandle
PoolHandle --> PooledClientLease
PooledClientLease --> WireframeClient
Envelope ..|> Packet
Envelope ..|> CorrelatableFrame
Packet --> PacketParts
Class diagram for testing utilities: wireframe::testkit and wireframe_testingclassDiagram
class TestSerializer {
+serialize<T>(value: &T) Result~Vec_u8,TestError~
+deserialize<T>(bytes: &[u8]) Result~T,TestError~
}
class TestResult~T~ {
<<type_alias>>
}
class TestError {
<<enum>>
Io(std_io_Error)
Codec(String)
Assertion(String)
}
class SlowIoPacing {
+chunk_size: NonZeroUsize
+interval: Duration
+new(chunk_size: NonZeroUsize, interval: Duration) SlowIoPacing
}
class SlowIoConfig {
+writer: Option~SlowIoPacing~
+reader: Option~SlowIoPacing~
+capacity: usize
+new() SlowIoConfig
+with_writer_pacing(pacing: SlowIoPacing) SlowIoConfig
+with_reader_pacing(pacing: SlowIoPacing) SlowIoConfig
+with_capacity(capacity: usize) SlowIoConfig
}
class MessageAssemblySnapshot {
+latest_key() Option~MessageKey~
+buffered_count() usize
+total_buffered_bytes() usize
}
class FragmentReassemblySnapshot {
+buffered_messages() usize
}
class TestkitDrivers {
<<module wireframe::testkit>>
+drive_with_partial_frames(app: WireframeApp, codec: FrameCodec, payloads: Vec~Vec_u8~, chunk: NonZeroUsize) Future~Output = TestResult~Vec~Vec_u8~~~
+drive_with_partial_frames_with_capacity(app: WireframeApp, codec: FrameCodec, payloads: Vec~Vec_u8~, chunk: NonZeroUsize, capacity: usize) Future~Output = TestResult~Vec~Vec_u8~~~
+drive_with_partial_frames_mut(app: &mut WireframeApp, codec: FrameCodec, payloads: Vec~Vec_u8~, chunk: NonZeroUsize) Future~Output = TestResult~Vec~Vec_u8~~~
+drive_with_partial_codec_frames(app: WireframeApp, codec: FrameCodec, payloads: Vec~Vec_u8~, chunk: NonZeroUsize) Future~Output = TestResult~Vec~Frame~~~
+drive_with_fragments(app: WireframeApp, codec: FrameCodec, fragmenter: Fragmenter, payload: Vec_u8) Future~Output = TestResult~Vec~Vec_u8~~~
+drive_with_fragments_with_capacity(app: WireframeApp, codec: FrameCodec, fragmenter: Fragmenter, payload: Vec_u8, capacity: usize) Future~Output = TestResult~Vec~Vec_u8~~~
+drive_with_fragments_mut(app: &mut WireframeApp, codec: FrameCodec, fragmenter: Fragmenter, payload: Vec_u8) Future~Output = TestResult~Vec~Vec_u8~~~
+drive_with_fragment_frames(app: WireframeApp, codec: FrameCodec, fragmenter: Fragmenter, payload: Vec_u8) Future~Output = TestResult~Vec~Frame~~~
+drive_with_partial_fragments(app: WireframeApp, codec: FrameCodec, fragmenter: Fragmenter, payload: Vec_u8, chunk: NonZeroUsize) Future~Output = TestResult~Vec~Vec_u8~~~
+drive_with_slow_frames(app: WireframeApp, bytes: Vec~u8~, config: SlowIoConfig) Future~Output = TestResult~Vec~u8~~~
+drive_with_slow_payloads(app: WireframeApp, payloads: Vec~Vec_u8~, config: SlowIoConfig) Future~Output = TestResult~Vec~u8~~~
+drive_with_slow_codec_payloads(app: WireframeApp, codec: FrameCodec, payloads: Vec~Vec_u8~, config: SlowIoConfig) Future~Output = TestResult~Vec~Vec_u8~~~
+drive_with_slow_codec_frames(app: WireframeApp, codec: FrameCodec, frames: Vec~Frame~, config: SlowIoConfig) Future~Output = TestResult~Vec~Frame~~~
}
class ReassemblyAssertions {
<<module wireframe::testkit::reassembly>>
+assert_message_assembly_incomplete(snapshot: MessageAssemblySnapshot) TestResult~()~
+assert_message_assembly_completed(snapshot: MessageAssemblySnapshot, expected_body: &[u8]) TestResult~()~
+assert_message_assembly_completed_for_key(snapshot: MessageAssemblySnapshot, key: MessageKey, expected_body: &[u8]) TestResult~()~
+assert_message_assembly_error(snapshot: MessageAssemblySnapshot) TestResult~()~
+assert_message_assembly_buffered_count(snapshot: MessageAssemblySnapshot, expected: usize) TestResult~()~
+assert_message_assembly_total_buffered_bytes(snapshot: MessageAssemblySnapshot, expected: usize) TestResult~()~
+assert_message_assembly_evicted(snapshot: MessageAssemblySnapshot, key: MessageKey) TestResult~()~
+assert_fragment_reassembly_absent(snapshot: FragmentReassemblySnapshot) TestResult~()~
+assert_fragment_reassembly_completed_len(snapshot: FragmentReassemblySnapshot, expected_len: usize) TestResult~()~
+assert_fragment_reassembly_error(snapshot: FragmentReassemblySnapshot) TestResult~()~
+assert_fragment_reassembly_buffered_messages(snapshot: FragmentReassemblySnapshot, expected: usize) TestResult~()~
+assert_fragment_reassembly_evicted(snapshot: FragmentReassemblySnapshot) TestResult~()~
}
class WireframePair {
+client_mut() Result~&mut WireframeClient,TestError~
+local_addr() SocketAddr
+shutdown() Future~Output = TestResult~()~~
}
class WireframeClient {
}
class PairFunctions {
<<module wireframe_testing::client_pair>>
+spawn_wireframe_pair(factory: F, builder_customizer: BuilderFn) Future~Output = TestResult~WireframePair~~
+spawn_wireframe_pair_default(factory: F) Future~Output = TestResult~WireframePair~~
}
class ObservabilityHandle {
+new() ObservabilityHandle
+clear() void
+recorder() MetricsRecorder
+snapshot() void
+counter(name: &str, labels: &[(&str,&str)]) u64
+counter_without_labels(name: &str) u64
+codec_error_counter(error_type: &str, recovery_policy: &str) u64
+assert_counter(name: &str, labels: &[(&str,&str)], expected: u64) TestResult~()~
+assert_no_metric(name: &str) TestResult~()~
+assert_codec_error_counter(error_type: &str, recovery_policy: &str, expected: u64) TestResult~()~
+assert_log_contains(substring: &str) TestResult~()~
+assert_log_at_level(level: &str, substring: &str) TestResult~()~
}
class MetricsRecorder {
}
class Labels {
+new() Labels
+with_label(key: &str, value: &str) Labels
}
class HotlineFrameCodec {
+new(max_frame_len: usize) HotlineFrameCodec
}
class HotlineFrame {
}
class HotlineFixtures {
<<module wireframe_testing::fixtures::hotline>>
+valid_hotline_wire(payload: &[u8], transaction_id: u32) Vec~u8~
+valid_hotline_frame(payload: &[u8], transaction_id: u32) HotlineFrame
+oversized_hotline_wire(max_frame_length: usize) Vec~u8~
+mismatched_total_size_wire(payload: &[u8]) Vec~u8~
+truncated_hotline_header() Vec~u8~
+truncated_hotline_payload(payload_len: usize) Vec~u8~
+correlated_hotline_wire(transaction_id: u32, payloads: &[&[u8]]) Vec~u8~
+sequential_hotline_wire(base_transaction_id: u32, payloads: &[&[u8]]) Vec~u8~
+new_test_codec() HotlineFrameCodec
}
class WireframeApp {
}
class FrameCodec {
<<trait>>
}
class Fragmenter {
+new(max_fragment_size: NonZeroUsize) Fragmenter
}
class Frame {
}
TestkitDrivers --> WireframeApp
TestkitDrivers --> FrameCodec
TestkitDrivers --> SlowIoConfig
TestkitDrivers --> Fragmenter
TestkitDrivers --> TestResult
ReassemblyAssertions --> MessageAssemblySnapshot
ReassemblyAssertions --> FragmentReassemblySnapshot
ReassemblyAssertions --> TestResult
PairFunctions --> WireframePair
WireframePair --> WireframeClient
ObservabilityHandle --> MetricsRecorder
ObservabilityHandle --> Labels
HotlineFixtures --> HotlineFrameCodec
HotlineFixtures --> HotlineFrame
TestSerializer --> TestResult
TestResult --> TestError
SlowIoConfig --> SlowIoPacing
MessageAssemblySnapshot --> MessageKey
FragmentReassemblySnapshot --> Frame
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Add a docs contents index and repository layout reference so contributors can find documentation quickly and understand the project structure. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Update the wireframe dependency to v0.3.0, repair the moved import and codec APIs, and switch the wireframe app factory to fail closed when handshake context is missing. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add roadmap follow-up tasks for explicit memory budgets, client streaming, and v0.3.0 test and observability tooling so the migration guide's new capabilities are tracked beyond the compatibility bump. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5237e9977c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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/CONTENTS.md`:
- Around line 3-4: Replace the non-Oxford en-GB-oxendict spellings in the
CONTENTS entries: change "organised" to "organized" in the opening sentence,
"Organisation" to "Organization" in the repository-layout bullet, and
"recognising" to "recognizing" in the
complexity-antipatterns-and-refactoring-strategies bullet; update the three
occurrences noted in the diff (the opening sentence and the two listed bullets)
so the document consistently uses "-ize" spellings.
- Line 8: Replace the sentence "Start here if you are new to the project." with
a third-person phrasing to remove second-person pronouns; change it to "Start
here for newcomers to the project." so the documentation follows the
no‑2nd‑person style rule.
In `@docs/execplans/wireframe-v0-3-0-migration.md`:
- Around line 105-107: Update the closeout checklist in the ExecPlan section
that currently lists only code gates so the migration requires documentation
gates as well: add `make fmt`, `make markdownlint`, and `make nixie` to the
required validation line that reads "Run the full repository validation
suite..." (and replicate the same additions in the analogous checklist sections
noted in the comment ranges), ensuring the final outcome only marks the plan
complete once `make check-fmt`, `make lint`, `make test`, `make fmt`, `make
markdownlint`, and `make nixie` all pass.
- Around line 323-327: The document currently hard-codes a machine-local
worktree path in the "All commands run from:" block; replace the specific path
"/data/leynos/Projects/mxd.worktrees/wireframe-v0-3-0-migration" with a portable
placeholder such as "$REPO_ROOT" or the phrase "repository root" so the
preserved command transcript is not workstation-specific and remains accurate
for all readers.
In `@docs/repository-layout.md`:
- Around line 8-30: Update the fenced blocks in docs/repository-layout.md that
are labeled as ```text``` to use the required identifier ```plaintext``` (these
are the non-code tree/list blocks like the top-level repo tree and the other
occurrences at the noted ranges); replace each ```text``` fence with
```plaintext``` so the renderer and linter treat them as non-code/plain text and
keep identifiers consistent across the document.
- Around line 46-55: Update the repository layout docs to reflect that the
wireframe server is a directory, not a flat file: replace the entry referencing
src/server/wireframe.rs with a directory entry "wireframe/" and add a nested
"mod.rs" line (e.g., show "wireframe/ # Wireframe bootstrap" and "└──
mod.rs # Wireframe runtime entry") so readers are directed to
src/server/wireframe/mod.rs instead of the nonexistent src/server/wireframe.rs.
- Around line 3-4: The doc uses en-GB spellings "organisation" and "organised"
that conflict with the repo's en-GB-oxendict-with-ize convention; update every
instance of "organisation" to "organization" and "organised" to "organized"
(including the occurrences referenced in the diff and the other spans around
lines 32-35 and 57-59) so the document uses -ize spellings consistently; search
the file for the exact tokens "organisation" and "organised" and replace them
with "organization" and "organized" respectively, preserving surrounding
punctuation and capitalization.
In `@docs/roadmap.md`:
- Around line 267-272: Unescape the footnote labels [^1] and [^2] (remove the
backslashes) and update their link targets to be relative to this directory by
changing occurrences of [docs/design.md ...](docs/design.md#...) to point to
[design.md ...](design.md#...), i.e., edit the footnote definitions for [^1] and
[^2] so they use GitHub-flavored Markdown footnote syntax and link to
design.md#transaction-routing-middleware-december-2025 and
design.md#regression-verification-via-the-wireframe-binary-february-2026
respectively.
In `@docs/wireframe-users-guide.md`:
- Around line 742-745: The fenced block containing the sample output lines
starting with "wireframe_codec_errors_total{error_type="framing"...}" should use
the required plaintext fence language instead of "text"; update the code fence
marker to specify plaintext so Markdown tooling and repo linting recognize it as
non-code sample output and satisfy the guideline to "Always provide a language
identifier for fenced code blocks; use 'plaintext' for non-code text."
In `@docs/wireframe-v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 1139-1140: The docs contradict each other about PooledClientLease:
pick the forwarded-methods contract (do not implement Deref to WireframeClient)
and update both the migration guide and the users' guide to match; change any
statements that say "implements Deref to WireframeClient" to instead state that
PoolHandle/PooledClientLease forward common request methods (call, send,
receive) to the underlying WireframeClient for one-off calls, and add a short
note that the lease is cheaply cloneable and does not expose a direct
WireframeClient reference.
- Around line 107-113: Update the migration guide so its API examples match the
real v0.3.0 API: replace mentions of the non-existent into_payload() on
PacketParts and FragmentParts with the actual accessors payload() for read-only
access and into_parts() for consuming ownership (or alternatively implement an
into_payload() shim in the codec if you prefer the original docs). Also
reconcile the soft-pressure threshold discrepancy by choosing the canonical
value used in the users guide and making both guides consistent (prefer setting
both to 80% to match the users guide) and update the diagram/text accordingly.
- Around line 400-423: The docs conflict: the migration guide claims read pacing
triggers at 75% while the users' guide says 80%; inspect the runtime enforcement
in the WireframeApp/memory budgeting logic (search for MemoryBudgets,
memory_budgets, BudgetBytes, and any pacing/pressure constants or calculations
in the fragmentation/budget enforcement code) to determine the true
soft-read-pacing threshold used in v0.3.0, then update this migration guide text
(and the users' guide) to the verified percentage so both files consistently
state the actual enforced threshold.
In `@src/server/wireframe/mod.rs`:
- Around line 74-85: AppFactoryError currently stores BuildApplication(String)
and the helper anyhow_from_error(stringifies errors), which flattens and drops
the original source chain from build_app; fix by preserving the source: change
BuildApplication to carry the original error type (e.g.,
BuildApplication(#[from] anyhow::Error) or a boxed dyn Error + Send + Sync) or
stop converting build_app errors to strings and instead call .context(...) /
with_context(...) on the Result returned by build_app where it's invoked; remove
or rewrite anyhow_from_error to accept/forward the original error (or eliminate
its use) and update the call sites that create AppFactoryError (or use
anyhow::Context directly) so the source is kept and the backtrace/chain is
preserved (refer to AppFactoryError::BuildApplication, anyhow_from_error, and
the places that call build_app).
In `@src/server/wireframe/tests.rs`:
- Around line 64-74: The test app_factory_rejects_missing_handshake_context can
be flakey due to leftover thread-local handshake state; before calling
build_app_for_connection, call take_current_context() to clear any existing
handshake context so the test fails closed reliably. Add a preamble in the test
(inside app_factory_rejects_missing_handshake_context) that invokes
take_current_context() and discards its return value, then proceed to create
pool/argon2/outbound_registry and call build_app_for_connection as before.
🪄 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: 0dd88248-ff7d-47a5-92b7-cf92366596a7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.tomldocs/CONTENTS.mddocs/execplans/wireframe-v0-3-0-migration.mddocs/repository-layout.mddocs/roadmap.mddocs/wireframe-users-guide.mddocs/wireframe-v0-2-0-to-v0-3-0-migration-guide.mdsrc/server/wireframe/mod.rssrc/server/wireframe/tests.rssrc/wireframe/codec/frame.rssrc/wireframe/handshake.rssrc/wireframe/outbound.rssrc/wireframe/protocol.rs
- Change AppFactoryError::BuildApplication to use #[source] anyhow::Error instead of String to preserve error context - Remove anyhow_from_error helper function - Update validate_app_factory to use anyhow! macro directly - Update app_factory_rejects_missing_handshake_context test to pattern-match on AppFactoryError::MissingHandshakeContext - Update app_factory_builds_when_handshake_context_is_present test to assert that take_current_context() returns None after app construction - Add unit tests for HotlineFrameCodec::wrap_payload, frame_payload, max_frame_length, and round-trip payload verification - Remove 2nd person pronoun from docs/CONTENTS.md - Expand ORM acronym in docs/repository-layout.md Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@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. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 10130
No annulments were required; none of the original suggestions conflicted with en-GB-oxendict spelling conventions or introduced unnecessary cyclomatic complexity. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 58
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
docs/complexity-antipatterns-and-refactoring-strategies.md (2)
314-329:⚠️ Potential issue | 🟡 MinorRestore sequential list numbering.
The list items have been renumbered from
2.through4.to all1.. Ordered lists must use sequential numbering for proper semantic markup.📝 Proposed fix
1. **Identify Logical Chunks:** Each "bump" or deeply nested section often corresponds to a specific sub-task or responsibility within the larger method.6 -1. **Extract to New Methods/Functions:** Encapsulate each identified chunk into +2. **Extract to New Methods/Functions:** Encapsulate each identified chunk into its own well-named method or function.8 The name of the new method should clearly describe its purpose. - This breaks down the large, complex function into smaller, more manageable, and understandable pieces.8 - Even if the overall Cognitive Complexity of the program doesn't change significantly, the complexity is spread out, making individual functions easier to grasp.8 -1. **Parameterize Extracted Methods:** Pass necessary data to the new methods as +3. **Parameterize Extracted Methods:** Pass necessary data to the new methods as parameters. Avoid relying on shared mutable state within the original class if possible, as this can maintain coupling. -1. **Iterative Refinement:** Refactoring complex code is often an iterative +4. **Iterative Refinement:** Refactoring complex code is often an iterative process. After initial extractions, further opportunities for simplificationTriage:
[type:syntax/md]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/complexity-antipatterns-and-refactoring-strategies.md` around lines 314 - 329, Fix the broken ordered list numbering in the "Extract to New Methods/Functions", "Parameterize Extracted Methods", and "Iterative Refinement" list items by changing the repeated "1." markers to a sequential "1.", "2.", "3." so the markdown renders as a proper ordered list; locate those headings/paragraphs in docs/complexity-antipatterns-and-refactoring-strategies.md and update the numeric prefixes accordingly (also check and correct any stray footnote markers like the trailing "8" if they were introduced accidentally).
485-524:⚠️ Potential issue | 🟡 MinorRestore sequential list numbering.
The list items have been renumbered from
2.through7.to all1.. Ordered lists must use sequential numbering for proper semantic markup.📝 Proposed fix
1. **Focus on Cohesion:** When extracting methods or creating new classes, ensure that the extracted code is functionally cohesive. Elements within a module (function or class) should be closely related and work together to achieve a single, well-defined purpose. Don't break down code arbitrarily based on length alone; base it on behaviour and meaningful abstractions.10 -1. **Balance Abstraction Levels:** Abstraction is about hiding unnecessary +2. **Balance Abstraction Levels:** Abstraction is about hiding unnecessary details and exposing essential features.27 - **Under-abstraction** (common in Spaghetti Code) leads to duplication and tight coupling.29 - **Over-abstraction** (risk in creating Ravioli Code) can make code harder to understand due to excessive layering and indirection, where simple operations are forced into complex object structures.24 - The key is to find the "right" level of abstraction that simplifies the problem domain without introducing unnecessary complexity. Create abstractions when painful duplication emerges or when a clear conceptual boundary can be established, not just for the sake of having more classes/objects.29 Start with simple, straightforward code and introduce abstractions only when genuinely needed.30 -1. **Meaningful Naming:** Clear and descriptive names for classes, methods, and +3. **Meaningful Naming:** Clear and descriptive names for classes, methods, and variables are crucial, especially when dealing with many small components. Good names help convey the purpose and relationships between different parts of the code.10 -1. **Consider the "Why," Not Just the "How":** When refactoring, understand the +4. **Consider the "Why," Not Just the "How":** When refactoring, understand the underlying responsibilities and collaborations. Simply breaking code into smaller pieces without a clear architectural vision can lead to Ravioli. Design patterns, when applied appropriately, can provide a "system metaphor" or structure that makes the "ravioli" manageable by revealing symmetries and common sense in the design.25 -1. **Iterative Refactoring and Review:** Refactoring is not always a one-shot +5. **Iterative Refactoring and Review:** Refactoring is not always a one-shot process. Continuously review the abstractions. Are they helping or hindering understanding? Are there too many trivial classes that could be consolidated?.10 Pair programming can also help maintain a balanced perspective during refactoring.25 -1. **The "You Aren't Gonna Need It" (YAGNI) Principle:** This principle helps +6. **The "You Aren't Gonna Need It" (YAGNI) Principle:** This principle helps avoid unnecessary abstractions and features, which can contribute to Ravioli code if abstractions are created for anticipated but not actual needs.25 -1. **Focus on System Flow:** While individual components in Ravioli code might +7. **Focus on System Flow:** While individual components in Ravioli code might be simple, the difficulty lies in tracing the overall execution flow. EnsureTriage:
[type:syntax/md]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/complexity-antipatterns-and-refactoring-strategies.md` around lines 485 - 524, The ordered list numbering was replaced with repeated `1.` entries, breaking semantic markup; update the sequence so the list items under the "Balance Abstraction Levels", "Meaningful Naming", "Consider the \"Why,\" Not Just the \"How\"", "Iterative Refactoring and Review", "The \"You Aren't Gonna Need It\" (YAGNI) Principle", and the final "Focus on System Flow" entries use proper ascending numbers (1., 2., 3., 4., 5., 6.) to restore correct ordered-list semantics; ensure any subsequent list continuation also follows sequential numbering and keep the existing text/order unchanged.docs/design.md (1)
107-151:⚠️ Potential issue | 🟡 MinorExecute
make markdownlintandmake nixieto validate this documentation before merge.The Makefile defines both targets and the Mermaid diagram is syntactically valid. Attach the passing output to the PR to satisfy the requirement that "all Markdown documents that include a
mermaidcode block must be validated using thenixieCLI before merging".Triage:
[type:syntax/md][type:docstyle]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design.md` around lines 107 - 151, Run the validation commands for the Mermaid diagram in docs/design.md: execute `make markdownlint` and `make nixie` locally (or in CI) to validate the mermaid `classDiagram` block (e.g., the NetworkRuntime/AppConfig/Commands diagram) and capture the full, passing CLI output; then attach those logs to the PR so the markdownlint/nixie validation requirement is satisfied before merge.docs/pg-embed-setup-unpriv-users-guide.md (1)
59-83:⚠️ Potential issue | 🟡 MinorFix ordered-step numbering in Quick start.
Renumber the sequence to
1.,2., and3.so the guide matches the enforced
ordered-list style and lint rules.Triage:
[type:syntax/md][type:docstyle]As per coding guidelines, "
docs/**/*.md: Follow markdownlint
recommendations for Markdown formatting" and "docs/**/*.md: Use '-' as the
first level bullet and renumber lists when items change".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/pg-embed-setup-unpriv-users-guide.md` around lines 59 - 83, The ordered list under "Quick start" currently repeats "1." for all steps; change the three list items so they are numbered sequentially as "1.", "2.", and "3." respectively (the block that starts with the Export configuration snippet, the paragraph beginning "Run the helper..." and the paragraph starting "Pass the resulting paths...") so the markdown follows the repo's ordered-list/lint rules and markdownlint recommendations.docs/execplans/1-4-2-route-transactions-through-wireframe.md (1)
403-462:⚠️ Potential issue | 🟠 MajorRenumber ordered lists sequentially in execution and acceptance steps.
Replace repeated
1.markers with1..Nnumbering to satisfy the configured
ordered-list style and avoidMD029failures.Triage:
[type:syntax/md][type:docstyle]As per coding guidelines, "
docs/**/*.md: Follow markdownlint recommendations
for Markdown formatting" and "docs/**/*.md: Use '-' as the first level
bullet and renumber lists when items change".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/1-4-2-route-transactions-through-wireframe.md` around lines 403 - 462, In the "Run the test suite" and "Validation and Acceptance" sections of the document (e.g., the numbered steps under "Run the test suite", "Run linting and formatting checks", "Run Markdown validation" and the "Validation and Acceptance" list), replace every repeated "1." list marker with sequential numbers (1., 2., 3., ...) so ordered lists are properly numbered and MD029 no longer triggers; also ensure top-level bullets use "-" per docstyle and reflow any affected lines so the Markdown linter (markdownlint) passes.docs/execplans/1-5-3-internal-compatibility-matrix.md (1)
256-294:⚠️ Potential issue | 🟡 MinorRenumber the concrete-step list sequentially to satisfy MD029.
Change the ordered list prefixes at Line 256, Line 262, Line 273, Line 280,
Line 285, and Line 294 from repeated1.to sequential values (2.through
7.after the first item).Triage:
[type:syntax/md][type:docstyle]As per coding guidelines, "Follow markdownlint recommendations for Markdown formatting" and "Use '-' as the first level bullet and renumber lists when items change."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/execplans/1-5-3-internal-compatibility-matrix.md` around lines 256 - 294, The ordered list in the "concrete-step" section uses repeated "1." prefixes; renumber the subsequent items so the list is sequential: change the prefixes for the items starting with "Extend automated tests where claims lack proof." then "Update documentation." then "Mark roadmap task done." then "Prepare local Postgres environment." and finally "Run quality gates with logs." to use 2., 3., 4., 5., 6., and 7. respectively (so the first "Add matrix document." remains 1. and the following items increment).
♻️ Duplicate comments (4)
docs/wireframe-v0-2-0-to-v0-3-0-migration-guide.md (1)
1086-1090:⚠️ Potential issue | 🟠 MajorUnify the
PooledClientLeasecontract statement.Remove the
derefclaim in the class-diagram caption or rewrite the later
paragraph; keep one contract only. The two passages currently contradict one
another.Triage:
[type:docstyle]As per coding guidelines, "Use markdown files within the
docs/directory as
a knowledge base and source of truth for project requirements, dependency
choices, and architectural decisions."Also applies to: 1136-1139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/wireframe-v0-2-0-to-v0-3-0-migration-guide.md` around lines 1086 - 1090, The class-diagram caption and the later paragraph disagree about PooledClientLease dereferencing to WireframeClient; pick one contract and make the docs consistent: either remove the "PooledClientLease derefs to WireframeClient" phrase from the class-diagram caption or rewrite the later paragraph to explicitly state that PooledClientLease implements/dereferences to WireframeClient (and update terminology to match), ensuring all mentions of PooledClientLease, WireframeClient, deref, and PooledClientLease contract are aligned across both locations (including the repeated lines at 1136-1139).docs/repository-layout.md (2)
50-52:⚠️ Potential issue | 🟠 MajorFix the
src/server/tree nesting.Indent
wireframe/mod.rsunderwireframe/. The current tree renders two
siblingmod.rsentries, which makes the layout internally inconsistent and
points readers at the wrong path.
Triage:[type:docstyle]🛠️ Proposed tree fix
│ ├── legacy.rs # Legacy networking runtime │ ├── wireframe/ # Wireframe bootstrap -│ └── mod.rs # Wireframe runtime entry +│ │ └── mod.rs # Wireframe runtime entry │ └── mod.rs # Server module entryBased on learnings, "Use markdown files within the
docs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/repository-layout.md` around lines 50 - 52, The docs tree currently shows two sibling "mod.rs" entries; fix the src/server/ tree by nesting the wireframe's entry under its directory: change the listing so "wireframe/" is followed by an indented "wireframe/mod.rs" (referenced as wireframe/mod.rs or "Wireframe runtime entry") instead of a sibling "mod.rs" at the same level as the server "mod.rs" (Server module entry); update the bullet/indentation in docs/repository-layout.md accordingly so the wireframe directory contains its own mod.rs entry.
32-35:⚠️ Potential issue | 🟡 MinorNormalize these headings and sentences to Oxford
-izespelling.
organisationandorganisedbreak the repository's en-GB-oxendict
convention. Useorganizationandorganizedhere.
Triage:[type:spelling][type:docstyle]🛠️ Proposed wording fix
-## Source code organisation (`src/`) +## Source code organization (`src/`) @@ -Documentation is organised by purpose and audience. +Documentation is organized by purpose and audience.Based on learnings, "Enforce en-GB oxendict spelling convention: prefer -ize endings (e.g., serialization, optimization, initialization) over -ise in codebase strings, comments, tests, docs, and identifiers where appropriate."
Also applies to: 58-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/repository-layout.md` around lines 32 - 35, Replace British -ise spellings in the stated section: change the heading "## Source code organisation (`src/`)" to "## Source code organization (`src/`)" and change the sentence "Documentation is organised by purpose and audience." to "Documentation is organized by purpose and audience."; also update the other occurrences noted around lines 58-60 to use "organization"/"organized" to enforce the en-GB oxendict -ize convention throughout the document.src/server/wireframe/mod.rs (1)
186-188:⚠️ Potential issue | 🟠 MajorStop stringifying app-build failures.
Both mappings create fresh
anyhow!values from formatted text, so the original
build_app()source chain is lost again. Preserve the underlying error in
AppFactoryError::BuildApplicationand layer context on top of it instead of
rebuilding the message.🔎 Verification script
#!/bin/bash set -euo pipefail sed -n '181,189p' src/server/wireframe/mod.rs printf '\n---\n' sed -n '265,283p' src/server/wireframe/mod.rs printf '\n---\n' rg -n 'BuildApplication|wireframe error:|failed to build wireframe application:' \ src/server/wireframe/mod.rsAs per coding guidelines, "Use
anyhow/eyrewith.context(...)in Rust to preserve backtraces and provide clear, typed failure paths."Also applies to: 281-283
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/wireframe/mod.rs` around lines 186 - 188, The code is replacing the original build_app error with a new anyhow! message; instead, preserve the original error and add context. Change the two map_err closures that currently do AppFactoryError::BuildApplication(anyhow!("wireframe error: {e}")) (the one after build_app(build_context) and the other at the later site) to wrap the incoming error using anyhow::Context (e.g. AppFactoryError::BuildApplication(e.context("wireframe error"))) or otherwise call .context("wireframe error") on the original error before passing it into AppFactoryError::BuildApplication so the original source chain/backtrace is preserved; locate the replacements around build_app, build_app_context and the AppFactoryError::BuildApplication occurrences.
🤖 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/adopting-hexagonal-architecture-in-the-mxd-wireframe-migration.md`:
- Around line 179-181: Remove the stray backslash escape characters that break
Markdown emphasis and reference rendering in the phrase containing “low-level
concerns” and the numeric reference [4]; replace the escaped emphasis sequence
(\*“low-level concerns” that would otherwise *“obscure the core application
logic”*\(...\)) with proper Markdown: use plain asterisks for emphasis
(*low-level concerns* and *obscure the core application logic*) and a clean
link/reference syntax like
[4](https://github.com/leynos/wireframe/blob/fa6c62925443e6caed54866a95d3396eb8fa78a2/docs/rust-binary-router-library-design.md#L5-L13);
apply the same fix to the other occurrence mentioned around the later block (the
instance flagged at line 252).
- Around line 117-118: Replace split ")(" citation constructs and inline numeric
links like "[1](https://...)" with GitHub-flavoured footnote references "[^1]"
and add corresponding footnote definitions in order (e.g., "[^1]: https://...")
at the document end or appropriate section; ensure numbering follows order of
appearance and update all affected occurrences (the ")(" token, inline
"[1](...)" patterns and their plain-text definitions) including the other listed
ranges so references and definitions are consistent and properly linked.
- Line 239: The sentence uses first-person phrasing ("we"); change it to an
impersonal/documentation voice by rewriting it to remove "we" — e.g., replace
"Conceptually, we can model this as a port interface like “ClientNotifier” with"
with an impersonal form such as "Conceptually, this can be modeled as a port
interface such as “ClientNotifier” with" or "Model this as a port interface like
“ClientNotifier” with", keeping the reference to the ClientNotifier port
interface.
In `@docs/adr-001-login-auth-reply-compatibility-layers.md`:
- Around line 114-117: The ordered list uses repeated "1." markers for later
items; update the list so Phase 2 and Phase 3 are numbered sequentially as "2."
and "3." respectively (i.e., change the lines beginning "Phase 2: Route login
handling through the strategy interface..." and "Phase 3: Add a SynHX hashed
authentication strategy..." to start with "2." and "3.") so the ordered list is
strictly sequential and markdownlint-compliant.
In `@docs/adr-002-compatibility-guardrails-and-augmentation.md`:
- Around line 102-107: Restore the ordered list numbering so the three migration
phases read "2.", "3.", and "4." respectively (the items currently all start
with "1."); update the three list items that reference Phase 2/Phase 3/Phase 4
in the docs/adr-002-compatibility-guardrails-and-augmentation.md content block
so the second item begins with "2.", the third with "3.", and the fourth with
"4." to preserve correct semantic ordered-list markup.
In `@docs/adr-003-login-authentication-and-reply-augmentation.md`:
- Around line 105-107: Fix the markdown ordered list numbering in ADR-003 by
changing the duplicate "1." entries for the migration plan phases to sequential
numbers ("2." for "Phase 2: Wire the strategy..." and "3." for "Phase 3: Add a
SynHX hashed authentication...") so the list entries are semantically correct;
update the two lines containing the phase headings in the
docs/adr-003-login-authentication-and-reply-augmentation.md file accordingly.
In `@docs/complexity-antipatterns-and-refactoring-strategies.md`:
- Around line 91-95: The ordered list numbering is incorrect—both items start
with "1."; update the markdown so the list uses sequential numbers (e.g., change
the item starting with "1. **Shorthand Discount:**" to "2." and any following
items to "3.", etc.) so the entries labeled "Nesting:" and "Shorthand Discount:"
render with proper semantic ordering.
- Around line 249-265: The ordered list in the document has every item numbered
"1." which breaks semantic numbering; update the list so each item uses
sequential numbers (e.g., "1.", "2.", "3.", "4.", "5.") corresponding to the
headings like "Incremental Refactoring", "Early Abstraction", "Code Reviews
Focused on Structure", "Using Complexity Metrics", and "Return Early / Guard
Clauses", preserving the existing text and punctuation for each item; after
fixing, run the markdown linter/preview to confirm the list renders as a proper
ordered list.
- Around line 347-370: The ordered list items in the markdown block (the entries
starting with "Deeply Nested Logic", "Functions Doing \"Too Much\"", "Frequent
Modifications to the Same Function...", "Difficulty in Unit Testing", "Code
\"Smells\" like Long Method", and "Declining Code Health Metrics") were all
prefixed with `1.` which breaks semantic ordered-list numbering; update these
list markers to consecutive numbers (1., 2., 3., 4., 5., 6.) so the sequence is
correct and preserves proper markdown ordered-list semantics.
In `@docs/design.md`:
- Around line 296-297: Replace first- and second-person phrasing in the sentence
"In MXD, we use Wireframe to handle all incoming TCP connections and route
decoded messages to handler functions based on their **transaction ID**." with a
neutral, declarative form (e.g., "MXD uses Wireframe to handle all incoming TCP
connections and route decoded messages to handler functions based on their
transaction ID."); apply the same pronoun removal to the other affected prose
ranges (around 869-871 and 1114-1115) so all instances avoid "we", "you", "the
user", etc., and use neutral wording instead.
- Around line 28-30: The inline numeric pseudo-citations like
"table([2](...))([1](...))" should be converted to Markdown footnote references
(e.g., replace the bracketed numeric links such as "[2]" and "[1]" with [^label]
tokens) and add corresponding footnote definitions at the end of the document
mapping each [^label] to the full GitHub URL using standard Markdown link
syntax; also find the plain path text referenced at 3587 and change it to a
clickable inline link (e.g., [text](url) or <url>) so it becomes navigable.
Ensure each footnote has a unique label and the original target URLs are
preserved exactly in the footnote definitions.
- Around line 561-574: The ordered lists in the design doc are using "1." for
every item rather than sequential numbering; update each affected ordered list
so items are numbered sequentially (1., 2., 3., …). Specifically, find the list
that begins with "Unit tests (`src/wireframe/codec/tests.rs`)" and renumber its
items to 1, 2, 3, 4; apply the same fix to the other changed sections that
mirror this pattern (the block that includes the BDD scenarios and parity
scenarios and the property tests, and the other occurrences around the later
sections referenced in the review). Keep the numbering explicit (do not switch
to auto-numbering shortcuts) and preserve the existing text and parentheses for
each item.
In `@docs/documentation-style-guide.md`:
- Line 312: The footnote line currently has an escaped bracket sequence
"\[^1\]:" which prevents GitHub-flavored Markdown from rendering it; update the
literal string in the documentation to use the correct footnote syntax "[^1]:"
(replace "\[^1\]:" with "[^1]:") so the in-text reference resolves properly in
the Markdown renderer.
In `@docs/execplans/1-2-4-model-handshake-readiness.md`:
- Around line 356-363: In the "Verification" section's ordered list (the three
items that start with "1. **Invariants documented:**", "1. **Integration test
passes:**", and "1. **Quality gates pass:**"), fix the markdown numbering so the
items are sequential (change the second and third "1." markers to "2." and "3.",
and if there's a fourth change to "4.") to satisfy markdownlint; update the
three list markers directly in docs/execplans/1-2-4-model-handshake-readiness.md
within the Verification section so the numbered list reads 1., 2., 3. (and 4. if
present).
In `@docs/execplans/1-3-4-kani-harnesses-for-transaction-framing-invariants.md`:
- Around line 190-234: The ordered steps in
docs/execplans/1-3-4-kani-harnesses-for-transaction-framing-invariants.md use
repeated "1." instead of sequential numbering; edit the markdown to renumber the
list items 1..N (e.g., 1., 2., 3., ...) for the seven steps referencing Kani
harness scaffolding, implementation files (src/transaction/reader/mod.rs,
src/wireframe/codec/mod.rs, src/header_util.rs), docs updates,
formatting/lint/run commands, and roadmap marking; ensure top-level list items
use '-' for bullets where required by docs/ style and run markdownlint/Makefile
checks to confirm the file passes the docs/**/*.md rules.
- Around line 46-62: Replace the fenced triple-backtick blocks that contain
pseudo-list text (the three "Risk:" entries and similar blocks) with proper
Markdown lists (use hyphen or checkbox syntax for each
Risk/Severity/Likelihood/Mitigation line) and remove the surrounding ``` fences
unless the content is an executable snippet; for any remaining fenced blocks
that are non-code, add an explicit language tag (use "plaintext") per docs
guidelines; apply the same change to the other similar fenced blocks mentioned
in the review so all lists are real Markdown lists and all fenced blocks have
explicit language identifiers.
In `@docs/execplans/1-4-2-route-transactions-through-wireframe.md`:
- Around line 398-401: The fenced code blocks in the ExecPlan (e.g., the block
containing "mkdir -p docs/execplans" and the block containing the comment "Write
this ExecPlan to docs/execplans/1-4-2-route-transactions-through-wireframe.md")
lack language identifiers; update those fences to include explicit languages
(use bash for shell commands like mkdir and plaintext for non-code text) and
apply the same change to the other listed blocks (around the snippets referenced
at 405-407, 413-416, 423-427, 433-436, 477-481, 485-490) so every
triple-backtick fence has an appropriate language tag.
In `@docs/execplans/1-4-3-introduce-a-shared-session-context.md`:
- Around line 215-221: The fenced code blocks in this document lack language
identifiers; update each triple-backtick block (for example the block containing
the Session struct and its fields: Session, Privileges, ConnectionFlags) to
include the appropriate language tag (use ```rust for Rust snippets, ```bash for
shell examples, and ```plaintext for non-code text) so the markdown linter
passes and accessibility is preserved; apply the same change to all other listed
blocks (312-320, 324-327, 330-333, etc.) ensuring each fence has the correct
language token.
- Around line 97-110: The Markdown ordered lists in
docs/execplans/1-4-3-introduce-a-shared-session-context.md use repeated "1."
markers; update these to sequential numbering and ensure first-level bullets use
"-" per style guide — specifically renumber the list entries that mention
authenticate(), setup_full_db, setup_files_db, setup_news_db,
ensure_test_user(), default_user(), NEWS_POST_ARTICLE and the PostNewsArticle
test; apply the same renumbering/first-level "-" rule for the other affected
ranges (lines referenced in the review: 118-125, 233-305, 310-424) so all
ordered lists are ascending and top-level bullets are "-" throughout the
document.
In `@docs/execplans/1-4-4-outbound-transport-and-messaging-traits.md`:
- Around line 255-286: The fenced code block containing the Rust types and
traits (OutboundPriority, OutboundConnectionId, OutboundTarget,
OutboundTransport, OutboundMessaging, send_reply, push, broadcast) is missing a
language identifier; update the opening fence from ``` to ```rust (or
```plaintext if intentionally non-compilable) so the block is marked correctly
for syntax highlighting and to satisfy repo docs rules.
- Around line 66-77: The checklist is currently inside a fenced code block so
GitHub won't render editable task checkboxes; remove the surrounding
triple-backtick fence and leave the lines starting with "- [x]" (the timestamped
items such as "(2026-01-17 02:03Z) Capture current outbound flows..." and the
other dated entries) as plain Markdown list items so they render as a Task List,
and then run markdownlint/formatting checks to ensure spacing and line breaks
conform to the project's Markdown rules.
- Around line 190-224: In the markdown steps block starting with the "Discovery"
item (the numbered list that currently repeats "1." for every entry), renumber
the list so the items are sequential (1. Discovery, 2. Add outbound traits and
tests, 3. Wireframe adapter plumbing, 4. Tests and documentation, 5. Formatting
and verification) and replace any other repeated "1." markers with the
appropriate ascending numbers to follow the project docstyle (use "-" for
top-level bullets elsewhere if applicable); edit the section containing the
lines "Discovery", "Add outbound traits and tests", "Wireframe adapter
plumbing", "Tests and documentation", and "Formatting and verification" to
reflect the new sequential numbering.
In `@docs/execplans/1-4-5-reply-builder.md`:
- Around line 280-291: The fenced code block showing the ReplyBuilder snippet
lacks a language identifier; update the Markdown fence to include a Rust
language tag (e.g., change the opening fence to "~~~rust" or "```rust") so the
block containing ReplyBuilder, FrameHeader, SocketAddr, and methods like
from_frame, from_header, and error_reply is lint-compliant and renders with Rust
syntax highlighting.
- Line 220: The ordered list in the concrete-steps section currently repeats
"1." and must be renumbered sequentially to "1, 2, 3, 4, 5"; update the list
items under the concrete-steps heading accordingly and ensure the corresponding
repeated items at the other occurrences referenced (lines around 225, 231, 237)
are also renumbered to maintain a proper 1..5 sequence so the document follows
markdownlint/Markdown formatting rules.
In `@docs/execplans/1-4-6-model-routed-transactions-and-session-gating.md`:
- Around line 277-280: Several fenced code blocks in the document (the
dependency block and multiple command blocks) lack language identifiers; update
the TOML dependency block to use ```toml and all shell/command blocks (cargo
check/test, make commands, and the "Run the full quality gate suite" block) to
use ```shell (or ```plaintext if any block is non-code text) so markdown syntax
highlighting follows guidelines; locate the specific fenced blocks around the
dependency list and the command examples (the cargo check -p mxd-verification,
cargo test -p mxd-verification -- session_gating, and the various make ...
commands) and add the appropriate language identifiers to each opening fence.
- Around line 375-385: Several fenced code blocks showing Rust examples (e.g.,
the privilege bit constants like DOWNLOAD_FILE, READ_CHAT, SEND_PRIVATE_MESSAGE;
the default user privileges composite; session privilege error codes; the
SessionModel struct and its fields; state and action types; and properties) are
missing the rust language identifier; update each relevant triple-backtick fence
before those snippets to start with ```rust so the blocks containing symbols
such as DOWNLOAD_FILE, CHANGE_OWN_PASSWORD, SessionModel, and the state/action
type names are properly marked for Rust syntax highlighting and tooling.
- Around line 294-300: The fenced code blocks that contain file path listings
(e.g., the block showing "crates/mxd-verification/src/session_model/mod.rs" and
the other session_model files) and the block with test-run output (e.g., the
"running 4 tests" / "test session_model_verifies_with_default_config … ok"
lines) are missing language identifiers; update those fenced blocks to use
```plaintext so they follow the guideline to mark non-code text as plaintext,
ensuring all occurrences (the file-path list block around the session_model
entries and the command output block showing the test run) are updated.
- Around line 136-142: The fenced code block containing the Session struct (the
declaration with pub struct Session and fields like pub user_id, pub privileges,
pub connection_flags) lacks a language identifier; update the opening fence to
include "rust" (i.e., ```rust) so the Session struct and its fields are
syntax-highlighted and properly recognized by tooling.
- Around line 158-161: The ordered list under the "three-tier verification
approach" currently uses three repeated "1." entries; update the three list
items in that section so they are sequentially numbered "1. 2. 3." (i.e., change
the second and third "1." to "2." and "3.") to follow proper Markdown
ordered-list semantics and satisfy markdownlint; keep the existing item text
(TLA+, Stateright, Kani) and ensure blank lines around the list remain intact.
- Around line 407-410: The fenced code block containing the dependency entries
stateright and rstest-bdd should include a language identifier for proper syntax
highlighting; update the block delimiter to include "toml" (i.e., ```toml) so
the snippet with stateright = "0.30" and rstest-bdd = "0.3.2" is rendered as
TOML.
- Around line 352-354: The validation criteria list currently uses repeated "1."
markers (the lines starting with "State count > 100 (logged by the state space
test)" and "All properties (safety and reachability) satisfied") — change the
ordered list markers to sequential numbers (1., 2., 3.) so the three items are
numbered 1, 2, 3, and run markdownlint to verify formatting; adjust the list
that contains "State count > 100 (logged by the state space test)" and the
following items accordingly.
In `@docs/execplans/1-6-1-port-unit-and-integration-tests.md`:
- Line 305: In the "Implement binary-backed world helpers in `test-util`" steps,
replace repeated leading "1." markers with sequential numbers so the ordered
list is properly numbered (e.g., 1., 2., 3., ...) across the section;
specifically update each repeated list item referenced (lines near markers 305,
314, 324, 334, 339, 358, 362) to follow sequential ordering to satisfy
markdownlint list-prefix rules and maintain consistent numbering for the
concrete steps.
In `@docs/execplans/adopt-pg-embed-setup-v0-5-0.md`:
- Line 262: Renumber the Markdown ordered lists so items use ascending numbers
instead of repeated "1." — update the "execution sequence" and "commit plan"
ordered list blocks (and any other repeated-1 lists in this document) to use
sequential numbering (1., 2., 3., ...), ensuring the list items under those
headings are ordered correctly to satisfy markdownlint recommendations for
ordered lists.
In `@docs/execplans/adopt-rstest-bdd-v0-4-0.md`:
- Line 134: The ordered lists in Stages A–D currently use repeated "1." items
which violates markdownlint’s ordered-list-prefix rule; update each sequential
list block under the headings "Stage A", "Stage B", "Stage C", and "Stage D" so
the numbered items increment correctly (1., 2., 3., ...) instead of repeating
"1." for every line; ensure you update all occurrences referenced in the comment
(the list blocks near the examples in Stages A–D) so markdownlint passes.
In `@docs/execplans/adopt-rstest-bdd-v0-5-0.md`:
- Around line 147-149: The ordered list uses repeated "1." entries which breaks
markdownlint; update the three command lines (`make check-fmt | tee
/tmp/check-fmt-$(basename "$(pwd)")-$(git branch --show).out`, `make lint | tee
/tmp/lint-$(basename "$(pwd)")-$(git branch --show).out`, `make test | tee
/tmp/test-$(basename "$(pwd)")-$(git branch --show).out`) so they are
sequentially numbered 1, 2, 3 to conform to markdownlint and documentation
style.
In `@docs/execplans/afl-circular-dependency-issue.md`:
- Line 225: The ordered list entries in the markdown section that start with
repeated "1." (specifically the instruction line "Edit `fuzz/Dockerfile` so it
has one builder stage and one final runtime" and the other occurrences
mentioned) must be renumbered sequentially (2., 3., 4., 5., 6.) to satisfy
markdownlint ordered-list style; edit the markdown so each ordered item
increments correctly rather than repeating "1.", ensuring the other affected
lines (around the same block at the referenced occurrences) are updated to
follow sequential numbering.
- Around line 119-120: Replace the malformed mixed backtick sequence around the
error output (the fragment containing "error: unknown start of token" and the
inline backticks around `make test` and the rustc path) with a proper fenced
code block; specifically, turn that inline/mixed sequence into a ```plaintext
fenced block containing the three lines shown in the patch ("make lint fails...
error: unknown start of token: `", "make test fails... /root/.rustup/.../rustc",
and "is missing") and close the fence, and update the surrounding sentence to
read "Observation: the required repository commit gates are presently blocked by
environment/tooling issues unrelated to this Dockerfile change. Evidence:"
followed by the fenced block and then the existing "Impact..." sentence.
In `@docs/execplans/wireframe-v0-3-0-migration.md`:
- Around line 226-231: Rename the repeated "1." entries in each ordered list to
sequential numbers so lists are valid markdown (e.g., change the series listing
tests/wireframe_handshake_metadata.rs, tests/wireframe_transaction.rs,
tests/wireframe_xor_compat.rs, src/wireframe/codec/tests.rs,
src/wireframe/codec/framed_tests.rs, src/wireframe/handshake.rs tests from all
"1." to "1.", "2.", "3.", ...), and do the same for the other ordered-list
blocks mentioned; also make sure first-level bullets use '-' where required by
the docs guideline. Ensure each independent list is renumbered starting at 1 and
increments correctly.
In `@docs/file-sharing-design.md`:
- Line 343: Fix MD029 and MD013 by converting lazy "1." list numbering to
sequential numbering (1, 2, 3, ...) throughout the document (search for the
repeated "1." list blocks near the "Permission Check: Ensure the user has rights
to list/browse this folder." section and other list sections) and rewrap all
long paragraphs to 80 columns (address the reported lines for MD013) so the file
complies with markdownlint; alternatively, if you opt to allow lazy numbering,
update .markdownlintrc to set MD029: { "style": "one_or_ordered" } and still
rewrap the paragraphs to 80 columns to satisfy the docs wrapping rule.
In `@docs/migration-plan-moving-mxd-protocol-implementation-to-wireframe.md`:
- Around line 351-377: Add missing footnote definitions for the labels [^10],
[^20], [^26], [^28], and [^32] (or remove their inline references) to restore
valid Markdown: locate the existing footnotes block near the bottom of the
document and append entries like "[^10]: <URL or citation>" matching the style
of the other footnotes shown (e.g., [^1]..[^9]), ensuring each label exactly
matches the referenced tag in the body; if a reference is obsolete, remove the
corresponding [^label] usage instead of adding a definition.
In `@docs/ortho-config-users-guide.md`:
- Around line 479-486: The ordered list under the precedence section uses "1."
for every item; update the three list entries titled "Configuration file",
"Environment variables", and "Command‑line arguments" to use sequential
numbering (1., 2., 3.) so the list complies with markdownlint MD029 and displays
proper numeric precedence.
- Around line 396-431: Update the top-level ordered list in the "Attempts to
load a configuration file..." section so the five main items use sequential
numbering (1, 2, 3, 4, 5) instead of each starting with "1."; locate the five
top-level list entries beginning with "Attempts to load a configuration file.",
"Adds an environment provider...", "Adds a provider containing the CLI
values...", "Merges vector fields...", and "Attempts to extract the merged
configuration..." and change their numeric prefixes to 1–5 in order to satisfy
markdownlint MD029.
In `@docs/pg-embed-setup-unpriv-v0-5-0-migration-guide.md`:
- Around line 7-8: Fix the ordered list numbering in the markdown by replacing
the duplicate leading "1." tokens so the items read as sequential numbers (e.g.,
"1. fixes for outstanding issues; and" then "2. new features that require usage
changes to realize their benefit."); update the second list item currently
labeled "1. new features that require usage changes to realize their benefit."
to "2." so the ordered list is semantically correct.
- Around line 397-417: The ordered list items under the section that currently
shows "Cleanup-mode behaviour", "Send-safe fixture behaviour", "Recovery path",
and "Manual settings path" were all changed to `1.`; restore proper sequential
numbering so the four list entries read `1.`, `2.`, `3.`, `4.` respectively to
preserve semantic ordered-list markup; locate the block containing the headings
"Cleanup-mode behaviour", "Send-safe fixture behaviour", "Recovery path", and
"Manual settings path" and update the numeric prefixes accordingly.
In `@docs/protocol.md`:
- Around line 182-189: Replace the repeated "1." markers with sequential
numbering for the four checklist items so the list becomes 1., 2., 3., 4.;
specifically update the lines containing "Allocate big-endian helpers", "Never
reuse transaction IDs", "Graceful error path", and "Unit-test fragmentation" to
use ordered numbers in sequence to satisfy Markdown linting and the repo's
docstyle rules.
In `@docs/rstest-bdd-users-guide.md`:
- Around line 1780-1782: Reflow the long list lines that describe the API note
so each line is wrapped to an 80-column target; specifically break the sentence
referencing StepDefinitionRegistry::{steps_for_keyword, steps_for_file} and the
following lines about returning Arc<CompiledStepDefinition> and shared compiled
matcher/metadata into multiple wrapped lines (max ~80 chars) to match the doc
style; also apply the same 80-column wrapping to the nearby lines mentioned
(around the block at 1819-1821) so the markdown list items comply with the docs’
paragraph wrap guidelines.
In `@docs/rstest-bdd-v0-5-0-migration-guide.md`:
- Around line 215-216: The inline error sentence is missing spaces around "when
importing from", making it malformed; update the markdown so the line reads:
**Error:** `no \`sync_to_async\` in the root` when importing from
`rstest_bdd::sync_to_async` and keep the Fix line suggesting to update imports
to `rstest_bdd::async_step::sync_to_async`; ensure the backticked symbols
`sync_to_async`, `rstest_bdd::sync_to_async`, and
`rstest_bdd::async_step::sync_to_async` are correctly formatted and spaced per
en-GB-oxendict grammar.
In `@docs/rust-doctest-dry-guide.md`:
- Line 21: Restore real Markdown footnotes by removing the backslash escapes in
the documentation comments; replace occurrences like "block found in the
documentation comments\[^3\]:" with "block found in the documentation
comments[^3]:" and unescape any footnote definitions and references elsewhere
(the same change applies to the other reported locations around lines 533 and
623-660) so that `[^label]` is used as a proper footnote rather than literal
text.
- Line 29: Several ordered lists in the markdown use non-sequential numbering
(e.g., repeating "1."); update each ordered list in rust-doctest-dry-guide.md so
list items use sequential numbers (1, 2, 3, ...) instead of repeated "1."
patterns; locate the affected lists by the visible list headers such as the
"Code Generation" list entry and any subsequent enumerations and renumber their
items consecutively to satisfy markdownlint list-prefix rules, ensuring every
ordered block increments properly throughout the document.
In `@docs/rust-testing-with-rstest-fixtures.md`:
- Around line 550-553: Update the ordered list numbering so it is sequential to
satisfy markdownlint MD029: change the list item that begins "1. **Functional
Limitations:** `#[once]` fixtures..." to start with "2." and change the list
item that begins "1. **Attribute Propagation:** `rstest` macros currently..." to
start with "3."; ensure the rest of each item's content remains unchanged so the
two items read "2. **Functional Limitations:** ..." and "3. **Attribute
Propagation:** ...".
- Line 1349: Change the table entry for #[rstest] to improve the article and
flow by rewording it to "Marks a function as an `rstest`-managed test; enables
fixture injection and parameterization." — update the cell containing #[rstest]
so it uses "rstest-managed test" (with backticks around rstest) instead of the
original article construction.
In `@docs/supporting-both-sqlite3-and-postgresql-in-diesel.md`:
- Line 66: Update the table cell that currently reads "booleans | `BOOLEAN
DEFAULT FALSE` | declare as `BOOLEAN`; Diesel serialises to 0 / 1 so this is
fine" by changing "serialises" to "serializes" and inserting a comma before "so"
so the clause reads "...Diesel serializes to 0 / 1, so this is fine".
In `@docs/whitaker-users-guide.md`:
- Around line 33-38: The ordered list in docs/whitaker-users-guide.md has every
item starting with "1." which breaks semantic markdown; update the four list
entries (the lines referencing cloning the Whitaker repository, building the
lint libraries, creating the `whitaker` and `whitaker-ls` wrapper scripts, and
ensuring the pinned Rust toolchain) to use sequential numbering (1., 2., 3., 4.)
so the ordered list renders correctly.
In `@docs/wireframe-users-guide.md`:
- Around line 1003-1016: The ordered list items describing per-frame
enforcement, soft-limit read pacing, and hard-cap connection abort should be
sequentially numbered; update the three list prefixes currently all marked "1."
so they read "1.", "2.", and "3." respectively (edit the three bullet lines that
start each paragraph of those list items to use distinct numeric prefixes) to
satisfy markdownlint ordered-list style and the project's markdown formatting
guidelines.
- Around line 2523-2596: The footnote definitions are escaped (e.g. "\[^1\]:")
which breaks Markdown reference resolution; replace each escaped label pattern
"\[^n\]:" with an unescaped GitHub-flavoured footnote label "[^n]:" in the
definitions block (and also remove any stray backslashes from in-text footnote
references like "\[^n\]" so they become "[^n]") so the labels in the diff
(patterns such as "\[^1\]:", "\[^2\]:", ... , "\[^53\]:") match the
corresponding references elsewhere in the document.
In `@src/wireframe/connection.rs`:
- Around line 193-203: The function take_current_context currently calls
take_registry_context twice and always invokes registry removal even when the
context came from the registry; change it so we first attempt to take from the
task-local (CONNECTION_CONTEXT) into from_task_local, and if that yields Some,
call take_registry_context once to perform explicit registry cleanup and return
the task-local value, otherwise return the result of take_registry_context
directly; update the implementation of take_current_context to use an if let
Some(...) { let _ = take_registry_context(); return Some(ctx); } else { return
take_registry_context(); } pattern to avoid the redundant second registry
removal.
- Around line 205-207: Rename the misleading function registry_len to a
boolean-named function (e.g., registry_visible or can_see_registry) because it
returns 0/1 based on current_context().is_some(), update its signature and
#[must_use] attribute to the new name, replace all callers of registry_len with
the new name, and update any tests and assertions that expected a numeric
registry length to assert the boolean visibility (and adjust expectations
accordingly); use current_context() and its is_some() check unchanged when
implementing the renamed function.
In `@src/wireframe/handshake.rs`:
- Around line 79-83: The current code calls store_current_context() inside the
scope before write_handshake_reply() completes, which can leave a registry entry
if the write fails; move the call to store_current_context(context) to
immediately after write_handshake_reply(stream, HANDSHAKE_OK).await? returns
successfully so the registry is only updated on success, or if you prefer to
keep it before the write then wrap the write in a match/if let and call
take_current_context() to remove the registry entry on any error path; adjust
uses of scope_current_context(...) and ensure take_current_context() is invoked
on failure to avoid orphaned entries.
---
Outside diff comments:
In `@docs/complexity-antipatterns-and-refactoring-strategies.md`:
- Around line 314-329: Fix the broken ordered list numbering in the "Extract to
New Methods/Functions", "Parameterize Extracted Methods", and "Iterative
Refinement" list items by changing the repeated "1." markers to a sequential
"1.", "2.", "3." so the markdown renders as a proper ordered list; locate those
headings/paragraphs in
docs/complexity-antipatterns-and-refactoring-strategies.md and update the
numeric prefixes accordingly (also check and correct any stray footnote markers
like the trailing "8" if they were introduced accidentally).
- Around line 485-524: The ordered list numbering was replaced with repeated
`1.` entries, breaking semantic markup; update the sequence so the list items
under the "Balance Abstraction Levels", "Meaningful Naming", "Consider the
\"Why,\" Not Just the \"How\"", "Iterative Refactoring and Review", "The \"You
Aren't Gonna Need It\" (YAGNI) Principle", and the final "Focus on System Flow"
entries use proper ascending numbers (1., 2., 3., 4., 5., 6.) to restore correct
ordered-list semantics; ensure any subsequent list continuation also follows
sequential numbering and keep the existing text/order unchanged.
In `@docs/design.md`:
- Around line 107-151: Run the validation commands for the Mermaid diagram in
docs/design.md: execute `make markdownlint` and `make nixie` locally (or in CI)
to validate the mermaid `classDiagram` block (e.g., the
NetworkRuntime/AppConfig/Commands diagram) and capture the full, passing CLI
output; then attach those logs to the PR so the markdownlint/nixie validation
requirement is satisfied before merge.
In `@docs/execplans/1-4-2-route-transactions-through-wireframe.md`:
- Around line 403-462: In the "Run the test suite" and "Validation and
Acceptance" sections of the document (e.g., the numbered steps under "Run the
test suite", "Run linting and formatting checks", "Run Markdown validation" and
the "Validation and Acceptance" list), replace every repeated "1." list marker
with sequential numbers (1., 2., 3., ...) so ordered lists are properly numbered
and MD029 no longer triggers; also ensure top-level bullets use "-" per docstyle
and reflow any affected lines so the Markdown linter (markdownlint) passes.
In `@docs/execplans/1-5-3-internal-compatibility-matrix.md`:
- Around line 256-294: The ordered list in the "concrete-step" section uses
repeated "1." prefixes; renumber the subsequent items so the list is sequential:
change the prefixes for the items starting with "Extend automated tests where
claims lack proof." then "Update documentation." then "Mark roadmap task done."
then "Prepare local Postgres environment." and finally "Run quality gates with
logs." to use 2., 3., 4., 5., 6., and 7. respectively (so the first "Add matrix
document." remains 1. and the following items increment).
In `@docs/pg-embed-setup-unpriv-users-guide.md`:
- Around line 59-83: The ordered list under "Quick start" currently repeats "1."
for all steps; change the three list items so they are numbered sequentially as
"1.", "2.", and "3." respectively (the block that starts with the Export
configuration snippet, the paragraph beginning "Run the helper..." and the
paragraph starting "Pass the resulting paths...") so the markdown follows the
repo's ordered-list/lint rules and markdownlint recommendations.
---
Duplicate comments:
In `@docs/repository-layout.md`:
- Around line 50-52: The docs tree currently shows two sibling "mod.rs" entries;
fix the src/server/ tree by nesting the wireframe's entry under its directory:
change the listing so "wireframe/" is followed by an indented "wireframe/mod.rs"
(referenced as wireframe/mod.rs or "Wireframe runtime entry") instead of a
sibling "mod.rs" at the same level as the server "mod.rs" (Server module entry);
update the bullet/indentation in docs/repository-layout.md accordingly so the
wireframe directory contains its own mod.rs entry.
- Around line 32-35: Replace British -ise spellings in the stated section:
change the heading "## Source code organisation (`src/`)" to "## Source code
organization (`src/`)" and change the sentence "Documentation is organised by
purpose and audience." to "Documentation is organized by purpose and audience.";
also update the other occurrences noted around lines 58-60 to use
"organization"/"organized" to enforce the en-GB oxendict -ize convention
throughout the document.
In `@docs/wireframe-v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 1086-1090: The class-diagram caption and the later paragraph
disagree about PooledClientLease dereferencing to WireframeClient; pick one
contract and make the docs consistent: either remove the "PooledClientLease
derefs to WireframeClient" phrase from the class-diagram caption or rewrite the
later paragraph to explicitly state that PooledClientLease
implements/dereferences to WireframeClient (and update terminology to match),
ensuring all mentions of PooledClientLease, WireframeClient, deref, and
PooledClientLease contract are aligned across both locations (including the
repeated lines at 1136-1139).
In `@src/server/wireframe/mod.rs`:
- Around line 186-188: The code is replacing the original build_app error with a
new anyhow! message; instead, preserve the original error and add context.
Change the two map_err closures that currently do
AppFactoryError::BuildApplication(anyhow!("wireframe error: {e}")) (the one
after build_app(build_context) and the other at the later site) to wrap the
incoming error using anyhow::Context (e.g.
AppFactoryError::BuildApplication(e.context("wireframe error"))) or otherwise
call .context("wireframe error") on the original error before passing it into
AppFactoryError::BuildApplication so the original source chain/backtrace is
preserved; locate the replacements around build_app, build_app_context and the
AppFactoryError::BuildApplication occurrences.
🪄 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: 0f630441-58fb-475a-97d8-f92ecbac5e16
📒 Files selected for processing (52)
AGENTS.mdREADME.mddocs/CONTENTS.mddocs/adopting-hexagonal-architecture-in-the-mxd-wireframe-migration.mddocs/adr-001-login-auth-reply-compatibility-layers.mddocs/adr-002-compatibility-guardrails-and-augmentation.mddocs/adr-003-login-authentication-and-reply-augmentation.mddocs/complexity-antipatterns-and-refactoring-strategies.mddocs/design.mddocs/documentation-style-guide.mddocs/execplans/1-2-4-model-handshake-readiness.mddocs/execplans/1-3-4-kani-harnesses-for-transaction-framing-invariants.mddocs/execplans/1-4-2-route-transactions-through-wireframe.mddocs/execplans/1-4-3-introduce-a-shared-session-context.mddocs/execplans/1-4-4-outbound-transport-and-messaging-traits.mddocs/execplans/1-4-5-reply-builder.mddocs/execplans/1-4-6-model-routed-transactions-and-session-gating.mddocs/execplans/1-5-1-detect-clients-that-xor-encode-text-fields.mddocs/execplans/1-5-2-gate-protocol-quirks-on-the-handshake-sub-version.mddocs/execplans/1-5-3-internal-compatibility-matrix.mddocs/execplans/1-5-4-verify-xor-and-sub-version-compatibility.mddocs/execplans/1-5-6-split-login-authentication-strategies-from-reply-augmentation.mddocs/execplans/1-6-1-port-unit-and-integration-tests.mddocs/execplans/adopt-pg-embed-setup-v0-5-0.mddocs/execplans/adopt-rstest-bdd-v0-4-0.mddocs/execplans/adopt-rstest-bdd-v0-5-0.mddocs/execplans/afl-circular-dependency-issue.mddocs/execplans/wireframe-v0-3-0-migration.mddocs/file-sharing-design.mddocs/internal-compatibility-matrix.mddocs/migration-plan-moving-mxd-protocol-implementation-to-wireframe.mddocs/ortho-config-users-guide.mddocs/pg-embed-setup-unpriv-users-guide.mddocs/pg-embed-setup-unpriv-v0-5-0-migration-guide.mddocs/protocol.mddocs/repository-layout.mddocs/roadmap.mddocs/rstest-bdd-users-guide.mddocs/rstest-bdd-v0-5-0-migration-guide.mddocs/rust-doctest-dry-guide.mddocs/rust-testing-with-rstest-fixtures.mddocs/supporting-both-sqlite3-and-postgresql-in-diesel.mddocs/users-guide.mddocs/whitaker-users-guide.mddocs/wireframe-users-guide.mddocs/wireframe-v0-2-0-to-v0-3-0-migration-guide.mdsrc/server/wireframe/mod.rssrc/server/wireframe/tests.rssrc/wireframe/codec/frame.rssrc/wireframe/connection.rssrc/wireframe/handshake.rssrc/wireframe/handshake_bdd.rs
…ection context - Replace thread-local storage with Tokio task-local storage for ConnectionContext - Add task-ID keyed registry to mirror context for synchronous app factory use - Introduce scope_current_context API to manage context scope in async tasks - Update handshake and tests to use scoped task-local context - Move handshake BDD tests to separate handshake_bdd.rs module - Improve async context preservation across Tokio multi-threaded runtime Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
… factory tests Refactored app factory test functions to replace duplicated setup code with a shared helper function `run_factory_with_stored_context`. This reduces redundancy and improves readability of tests covering context handling in app factory. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/developers-guide.md`:
- Around line 106-107: Update the sentence describing `MissingPeerAddress` so it
reads "when the handshake metadata exists, but no peer address was attached." —
insert a comma before "but" in the clause that begins with "but no peer
address..." within the `MissingPeerAddress` description to separate the two
independent clauses.
In `@docs/wireframe-users-guide.md`:
- Around line 800-803: Replace the fenced block that starts with ```text and
contains the sample metrics (lines with
wireframe_codec_errors_total{error_type="framing",...} and
wireframe_codec_errors_total{error_type="eof",...}) so the fence language
identifier is `plaintext` instead of `text`; update any identical non-code
fenced blocks in the same document to use `plaintext` per the coding guideline.
In `@src/server/wireframe/tests.rs`:
- Around line 96-119: The test
app_factory_builds_when_handshake_context_is_present duplicates runtime and
scope_current_context boilerplate already in run_factory_with_stored_context;
refactor by adding a new helper (or extend run_factory_with_stored_context) that
accepts a closure for post-factory assertions so you can store the
ConnectionContext, call build_app_for_connection(&pool, &argon2,
&outbound_registry) inside the scoped async, then invoke the closure to run
take_current_context().is_none() (or other checks) after the factory runs;
reference run_factory_with_stored_context,
app_factory_builds_when_handshake_context_is_present, build_app_for_connection,
store_current_context, take_current_context, and scope_current_context when
locating where to add the new helper and call site changes.
🪄 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: f8c76ceb-2211-4166-aec5-0fd284f1b9f3
📒 Files selected for processing (4)
docs/developers-guide.mddocs/wireframe-users-guide.mdsrc/server/wireframe/mod.rssrc/server/wireframe/tests.rs
…sertions Refactor the app factory test helpers to use a common function that accepts context and assertions closure, improving code reuse and clarity. Adjust tests to assert context consumption after app factory runs. Also include minor docs fixes unrelated to test logic. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…ke context - Updated developers guide to reflect WireframeServer construction now using the `AppFactory` trait instead of a bare closure. - Added usage examples showing migration from returning `WireframeApp` to a fallible factory returning `Result<WireframeApp, AppFactoryError>`. - Enhanced users guide with example pattern for accessing handshake context inside app factories to support connection-specific validation and error propagation. - Added clarification of how handshake context can be retrieved and propagated as typed errors rather than panics. - Minor addition of doc comments in FrameCodec test module about test coverage. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
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/wireframe-users-guide.md`:
- Around line 2660-2661: Update the inconsistent ADR link targets for the
footnote(s) referring to ADR 0002: ensure the footnote reference(s) (e.g.,
[^46]) point to the same path pattern used elsewhere by changing the target to
docs/adr/0002-streaming-requests-and-shared-message-assembly.md for the
occurrences around the lines noted (including the block at 2686-2688 and the
reference at 2661/2687) so all links use the exact same ADR path.
In `@src/wireframe/codec/frame.rs`:
- Around line 121-156: Replace the duplicated unit tests in the tests module
with parameterized rstest cases: add use rstest::{fixture, rstest}; create a
fixture fn codec() -> HotlineFrameCodec { HotlineFrameCodec::new() } and replace
the two wrap_payload tests with a single #[rstest] function that takes (codec:
HotlineFrameCodec, #[case] bytes: Bytes, #[case] expected: Vec<u8>) and calls
codec.wrap_payload(bytes) asserting equality with expected; similarly replace
the two frame_payload tests with a single #[rstest] fn that takes #[case] data:
Vec<u8>, #[case] expected: Vec<u8> and asserts
HotlineFrameCodec::frame_payload(&data) == expected.as_slice(); ensure rstest is
added to dev-dependencies if not present.
🪄 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: 182c01f3-bb94-4154-a05f-fabcf0f607cc
📒 Files selected for processing (4)
docs/developers-guide.mddocs/wireframe-users-guide.mdsrc/server/wireframe/tests.rssrc/wireframe/codec/frame.rs
…test comments Add Markdown-style backticks around `HotlineFrameCodec` in the test module documentation comments to improve clarity and formatting. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Wireframe v0.3.0 adapts `FrameCodec::wrap_payload` to take `Bytes` instead of `Vec<u8>`. - Module imports reorganized to prefer explicit module paths over root-level re-exports. - Migration guide added to developers' documentation covering these changes. - Refactor tests for `HotlineFrameCodec` using rstest fixtures and parameterized cases. - Revise handshake and runtime selection BDD tests to properly assert expected behavior. - Correct link in user guide documentation. These changes improve the codec interface for zero-copy optimizations, clarify usage patterns, and enhance test robustness and maintainability. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please suggest a fix for the following lint violations and provide an AI coding agent prompt for the fix: |
This comment was marked as resolved.
This comment was marked as resolved.
Refactored news-related command parsing code by introducing dedicated parsing functions for NewsCategoryNameList, NewsArticleNameList, NewsArticleData, and PostNewsArticle. This improves code clarity, reusability, and maintainability by consolidating parameter decoding and extraction logic. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/wireframe/handshake_bdd.rs`:
- Around line 135-138: In when_custom, validate that tag.len() == 4 before
calling protocol.copy_from_slice; if not, return Err with a clear message so the
step fails cleanly instead of panicking — add the length check and early return
in the when_custom function that prepares protocol and calls preamble_bytes,
referencing the protocol buffer, tag parameter, and preamble_bytes call so the
guard sits immediately before copy_from_slice.
🪄 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: 80a43161-4b1a-49e9-a65f-0e29a4f912d6
📒 Files selected for processing (6)
docs/developers-guide.mddocs/wireframe-users-guide.mdsrc/commands/mod.rssrc/wireframe/codec/frame.rssrc/wireframe/handshake_bdd.rstests/runtime_selection_bdd.rs
Add a check to ensure the protocol tag length is exactly 4 bytes before sending a Hotline handshake. This prevents potential runtime errors from invalid tag sizes. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/wireframe/handshake_bdd.rs (1)
62-64:⚠️ Potential issue | 🟠 MajorMake the harness fallible end-to-end.
Line 64 still panics during fixture setup, and Lines 86-88 still return
Ok(())after a failed handshake by stashing theErrinreply. Propagate these failures through the fixture and step return path soassert_step_ok!fails at the actual failing step.Patch
impl HandshakeWorld { - fn new() -> Self { - Self { - rt: Runtime::new().expect("runtime"), + fn try_new() -> Result<Self, String> { + Ok(Self { + rt: Runtime::new().map_err(|err| err.to_string())?, addr: RefCell::new(None), shutdown: RefCell::new(None), reply: RefCell::new(None), - } + }) } @@ fn connect_and_maybe_send(&self, bytes: Option<Vec<u8>>) -> Result<(), String> { let addr = self .addr @@ .as_ref() .copied() .ok_or_else(|| "server not started".to_string())?; - let reply = self.rt.block_on(perform_handshake(addr, bytes)); - self.reply.borrow_mut().replace(reply); + let reply = self.rt.block_on(perform_handshake(addr, bytes))?; + self.reply.borrow_mut().replace(Ok(reply)); Ok(()) } } @@ #[fixture] -fn world() -> HandshakeWorld { HandshakeWorld::new() } +fn world() -> Result<HandshakeWorld, String> { HandshakeWorld::try_new() }As per coding guidelines, "In production code and shared fixtures, avoid
.expect()entirely: returnResultand use?to propagate errors instead of panicking." and "Consume fallible fixtures inrstestby making the test returnResultand applying?to the fixture."Also applies to: 79-88, 122-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wireframe/handshake_bdd.rs` around lines 62 - 64, The fixture currently panics and hides handshake failures: remove the panic in fn new() by making Runtime::new() fallible (change fn new() -> Self to something like fn new() -> Result<Self, E>) and use ? instead of .expect("runtime"); stop stashing handshake errors into reply (the code around reply and the handshake handling in the handshake step at lines ~79-88) and instead propagate the Err through the fixture/step return types so the test function can return Result and use ?; likewise adjust any consumer of the fixture (and the step functions referenced at ~122-123) to return Result and bubble up errors so assert_step_ok! fails at the actual failing step. Ensure unique symbols updated: new(), Runtime::new(), reply, handshake (step) and any step functions return Result and use ? to propagate errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/wireframe/handshake_bdd.rs`:
- Around line 62-64: The fixture currently panics and hides handshake failures:
remove the panic in fn new() by making Runtime::new() fallible (change fn new()
-> Self to something like fn new() -> Result<Self, E>) and use ? instead of
.expect("runtime"); stop stashing handshake errors into reply (the code around
reply and the handshake handling in the handshake step at lines ~79-88) and
instead propagate the Err through the fixture/step return types so the test
function can return Result and use ?; likewise adjust any consumer of the
fixture (and the step functions referenced at ~122-123) to return Result and
bubble up errors so assert_step_ok! fails at the actual failing step. Ensure
unique symbols updated: new(), Runtime::new(), reply, handshake (step) and any
step functions return Result and use ? to propagate errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c40b99ce-7c49-40ec-90ac-55ffb1c4b7a4
📒 Files selected for processing (1)
src/wireframe/handshake_bdd.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Migrate to wireframe v0.3.0 and adopt scoped task-local context with a fallible app factory. Context is carried through wiring rather than TLS and into the app factory; missing handshake context now fails closed and surfaces app-factory errors.
Enhancements:
Documentation:
Tests:
📎 Task: https://www.devboxer.com/task/c03db20c-c80d-44ef-a47b-0d19b243a551
📎 Task: https://www.devboxer.com/task/d2ea300b-4053-498c-9a59-8d51982e0aca
📎 Task: https://www.devboxer.com/task/fd9d959e-d88b-42ad-bcb4-d3361f4178d0