Repository navigation
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Environment lookup and address parsing Cargo.toml, examples/packet_enum.rs |
packet_enum uses the mockable environment interface and passes the SERVER_ADDR lookup result to its parser. Lookup errors use DEFAULT_ADDR; malformed configured addresses return InvalidInput. Tests cover missing and non-Unicode values, valid addresses, and malformed input. |
Example task and tracing updates
| Layer / File(s) | Summary |
|---|---|
Task results and tracing bindings examples/client_echo_login.rs, examples/metadata_routing.rs, examples/multi_packet.rs |
The examples bind tracing initialisation results and retain the prepared application in a named variable. The summary task logs a chunk-task join error and continues to send the summary. |
RESP codec const functions and parser names
| Layer / File(s) | Summary |
|---|---|
Codec constructors and parsing helpers examples/resp_codec_impl/codec.rs, examples/resp_codec_impl/encode.rs, examples/resp_codec_impl/parse.rs |
Selected constructors and helpers are now const fn. The parser uses named bulk payload and array element counts. Parsing limits and errors remain unchanged. |
Suggested reviewers: codescene-access
Priority:
Change: Refactor
Merge Risk: 
0cf5a
-
No concrete issue remains that needs resolution before merging; proceed with normal checks.
Caution
Pre-merge checks failed
Please resolve all errors before merging. Addressing warnings is optional.
- Ignore
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Testing (Overall) |  |
The new address parsing behaviour has substantive tests in examples/packet_enum.rs:144-179, covering missing, non-Unicode, valid and malformed values. However, the pull request also changes detached… |
Add a deterministic test for the multi-packet failure path. Inject or otherwise trigger a failing chunk task, capture the local tracing output, and assert that chunk producer failed and the join error are emitted. Keep the test failing wh… |
| Testing (Unit And Behavioural) | |
The new tests cover meaningful default, non-Unicode, valid, and malformed values, but they only call the private parse_server_addr helper (examples/packet_enum.rs:154-179). The changed example wor… |
Add a behavioural or end-to-end test for packet_enum that exercises the injected environment lookup through the server setup and verifies the selected listener address. Cover the fallback address, a configured valid address, and malformed… |
| Developer Documentation |  |
The pull request adds the mockable = "3.0.0" development dependency in Cargo.toml:78 and introduces an injectable environment boundary in examples/packet_enum.rs through mockable::Env and `Def… |
Update docs/developers-guide.md with the new development requirement and boundary. Document mockable = "3.0.0", the use of Env/DefaultEnv at the environment edge, pure parsing helpers that accept lookup results, and tests that avoid… |
| Testing (Compile-Time / Ui) | |
The pull request introduces compile-time behaviour through pub const fn RespFrameCodec::new and several const fn helpers in examples/resp_codec_impl/{codec,encode,parse}.rs. The existing tests c… |
Add a trybuild pass fixture and register it in the existing compile-time harness. Exercise RespFrameCodec::new in a const context, and cover the changed const helpers where their visibility permits. Keep assertions focused on successful c… |
-
Check name Status Explanation Title check The title accurately describes the two main changes: resolving example lint findings and adding an environment boundary. No roadmap or issue number is required because the description does not identif… Description check The description directly explains the environment boundary, lint remediation, dependency change, tests, validation evidence, and PR dependencies. Docstring Coverage Docstring coverage is 80.95% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 7 files. (1 skipped: 1 … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. User-Facing Documentation Pass this check. The pull request changes only example sources and test-only dependency metadata; it does not add or change library functionality. packet_enumpreserves the existingSERVER_ADDRfa…Module-Level Documentation PASS. Every changed top-level Rust module has module documentation explaining its purpose. The new packet_enum::testsmodule also has an inner docstring atexamples/packet_enum.rs:144-146. The und…Testing (Property / Proof) No property-test requirement is introduced. The behavioural change in examples/packet_enum.rshas a small, auditable contract: bothVarErrorvariants useDEFAULT_ADDR, a valid configured address…Unit Architecture No Unit Architecture failure is introduced. examples/packet_enum.rs:109-116keeps address parsing as a pure function with an explicit input andstd::io::Resultoutput.runperforms the environme…Domain Architecture Pass. Keep the environment lookup at the server setup boundary: runcallsDefaultEnv.raw("SERVER_ADDR")and passes the result toparse_server_addr(examples/packet_enum.rs:119-125). The parser o…Observability Pass the observability check. The change makes the detached chunk producer failure visible at its async task boundary with an error log and the stable operation context chunk producer failed(`examp…Full details: Title check
-
Explanation
The title accurately describes the two main changes: resolving example lint findings and adding an environment boundary. No roadmap or issue number is required because the description does not identify an issue or roadmap item that this PR implements.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 80.95% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 7 files. (1 skipped: 1 unsupported.)
Full details: User-Facing Documentation
-
Explanation
Pass this check. The pull request changes only example sources and test-only dependency metadata; it does not add or change library functionality.
packet_enumpreserves the existingSERVER_ADDRfallback and validation behaviour, while the other edits are lint, const, binding, and example logging changes. No update todocs/users-guide.mdis required.
Full details: Module-Level Documentation
-
Explanation
PASS. Every changed top-level Rust module has module documentation explaining its purpose. The new
packet_enum::testsmodule also has an inner docstring atexamples/packet_enum.rs:144-146. The undocumentedtestsmodules inexamples/resp_codec_impl/codec.rsandencode.rspre-date this pull request and were not changed, so they do not create a pull-request-caused failure.
Full details: Testing (Property / Proof)
-
Explanation
No property-test requirement is introduced. The behavioural change in
examples/packet_enum.rshas a small, auditable contract: bothVarErrorvariants useDEFAULT_ADDR, a valid configured address parses, and malformed input returnsInvalidInput; the added rstest and unit tests cover these branches. The remaining changes are const qualification, binding renames, and explicit task/result handling, with no new broad input or state invariant. Existing parser property tests are unchanged.
Full details: Unit Architecture
-
Explanation
No Unit Architecture failure is introduced.
examples/packet_enum.rs:109-116keeps address parsing as a pure function with an explicit input andstd::io::Resultoutput.runperforms the environment query at the composition boundary throughDefaultEnv.raw("SERVER_ADDR")and propagates lookup/parsing failure atexamples/packet_enum.rs:119-125. Tests supplyOkand bothVarErrorcases directly, without mutating process state (examples/packet_enum.rs:144-179). The remaining changes only clarify bindings, make pure helpersconst, and log an existing detached-task failure; they do not add hidden dependencies or unrelated responsibilities.
Full details: Domain Architecture
-
Explanation
Pass. Keep the environment lookup at the server setup boundary:
runcallsDefaultEnv.raw("SERVER_ADDR")and passes the result toparse_server_addr(examples/packet_enum.rs:119-125). The parser only converts the supplied value into aSocketAddrand applies the fallback (lines 108-117), so it does not access process environment state. The changed files do not add infrastructure dependencies to domain logic or alter a domain model boundary. The tests exercise the parser with explicit results (lines 144-180), which keeps the parsing logic independent of ambient environment state.
Full details: Observability
-
Explanation
Pass the observability check. The change makes the detached chunk producer failure visible at its async task boundary with an error log and the stable operation context
chunk producer failed(examples/multi_packet.rs:96-104). TheSERVER_ADDRchange preserves the existing default and malformed-input behaviour while adding isolated tests; it does not add a new production failure mode (examples/packet_enum.rs:108-125,144-180). The remaining edits are lint-oriented or binding renames and do not require new metrics, alerts, or tracing.
-
Full details: Testing (Overall)
-
Explanation
The new address parsing behaviour has substantive tests in
examples/packet_enum.rs:144-179, covering missing, non-Unicode, valid and malformed values. However, the pull request also changes detached-task error handling inexamples/multi_packet.rs:96-100by logging aJoinErrorinstead of discarding it. No test exercises this path; repository search found no test formulti_packet_responseorchunk producer failed. The implementation could revert to ignoring the join result and the tests would still pass.Resolution
Add a deterministic test for the multi-packet failure path. Inject or otherwise trigger a failing chunk task, capture the local tracing output, and assert that
chunk producer failedand the join error are emitted. Keep the test failing when the join result is discarded.
Full details: Testing (Unit And Behavioural)
-
Explanation
The new tests cover meaningful default, non-Unicode, valid, and malformed values, but they only call the private
parse_server_addrhelper (examples/packet_enum.rs:154-179). The changed example workflow readsSERVER_ADDRand bindsWireframeServerinrun(examples/packet_enum.rs:119-134). No test exercises that environment-to-bind boundary. The repository's TCP tests cover generic server behaviour, not this example wiring.Resolution
Add a behavioural or end-to-end test for
packet_enumthat exercises the injected environment lookup through the server setup and verifies the selected listener address. Cover the fallback address, a configured valid address, and malformed configuration at that boundary. Keep the existing helper unit tests for parser-specific error details.
Full details: Developer Documentation
-
Explanation
The pull request adds the
mockable = "3.0.0"development dependency inCargo.toml:78and introduces an injectable environment boundary inexamples/packet_enum.rsthroughmockable::EnvandDefaultEnv.raw. These are new development requirements and an internal testing abstraction. The pull request changes no file underdocs/, anddocs/developers-guide.mddoes not documentmockable, the environment boundary, or its testing convention. The separate dependency-injection document does not satisfy the explicit developer's-guide requirement.Resolution
Update
docs/developers-guide.mdwith the new development requirement and boundary. Documentmockable = "3.0.0", the use ofEnv/DefaultEnvat the environment edge, pure parsing helpers that accept lookup results, and tests that avoid mutating the process environment. Link the existing dependency-injection document if it remains normative, and record any related architectural decision in the relevant design document or ADR.
Full details: Testing (Compile-Time / Ui)
-
Explanation
The pull request introduces compile-time behaviour through
pub const fn RespFrameCodec::newand severalconst fnhelpers inexamples/resp_codec_impl/{codec,encode,parse}.rs. The existing tests call these functions at runtime only; they do not prove const-context use. The repository already uses trybuild intests/compile_error.rs, but the pull request adds no compile-time fixture or harness entry. The changed examples add no snapshot tests. The changed logging and parser error text do not establish a separate stable snapshot contract, so a focused snapshot is not required for this change.Resolution
Add a trybuild pass fixture and register it in the existing compile-time harness. Exercise
RespFrameCodec::newin a const context, and cover the changed const helpers where their visibility permits. Keep assertions focused on successful const evaluation. Retain the existing semantic error assertion forSERVER_ADDR; add a snapshot only if the example log or structured output becomes an intended stable contract.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Set SERVER_ADDR where addresses roam,
Let codec helpers count at compile time.
Log the task that could not join,
Keep each example’s bindings clear.
Send the final summary on its way.
Comment @coderabbitai help to get the list of available commands.
Reviewer's GuideThe PR remediates lint findings across workspace examples while introducing a mockable environment boundary for SERVER_ADDR parsing, adding isolated coverage for its fallback and validation behavior, and making selected RESP helpers const-evaluable. Sequence diagram for injected SERVER_ADDR parsingsequenceDiagram
participant Run as run
participant Env as DefaultEnv
participant Parser as parse_server_addr
participant Server as WireframeServer
Run->>Env: raw(SERVER_ADDR)
Env-->>Run: Result<String, VarError>
Run->>Parser: parse_server_addr(address_result)
alt lookup fails
Parser-->>Run: DEFAULT_ADDR
else configured address
Parser-->>Run: parsed SocketAddr
else malformed address
Parser-->>Run: InvalidInput error
end
Run->>Server: bind(socket_addr)
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Replace suppressed or silent conversions in the RESP and metadata examples with explicit, fallible forms, mark eligible example constructors `const`, and give the detached summary task a visible error path. `packet_enum` read `SERVER_ADDR` directly with `std::env::var` and mapped a missing value through `unwrap_or_else`. It now accepts the lookup result and reads it through `mockable::DefaultEnv`, so the environment is an injected boundary rather than ambient process access. Coverage for missing, non-Unicode, valid, and malformed values runs without mutating the environment. The new `mockable` dev-dependency is the only lockfile change. Co-Authored-By: Claude Code <noreply@anthropic.com>
53ee29b to
0cf5a8c
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Scope
Source remediation for the workspace examples, plus the environment boundary
they need. Nine paths change:
examples/packet_enum.rsnow readsSERVER_ADDRthroughmockable'sEnv/DefaultEnvinstead of ambientstd::env, and the parse is extractedinto a testable
parse_server_addr. Four unit tests cover the missing,non-Unicode, configured and malformed cases without mutating the process
environment.
examples/client_echo_login.rs,examples/metadata_routing.rs,examples/multi_packet.rsandexamples/resp_codec_impl/{codec,encode,parse}.rsreceive const-ification, distinct shadowed bindings, an error-visible
detached task, and an explicitly ignored subscriber result.
Cargo.toml/Cargo.lockaddmockable = "3.0.0"as a dev-dependency(ten lockfile lines, no existing version drift).
Position and dependencies
Layer 3 of 5. Stacked on the residual app layer, and the parent of the
self-named-module layer.
harden-lint-app-support-residual-20261022(Fix residual app and test-support lint findings #796)harden-lint-self-named-modules-20261022(Move self-named module files to mod.rs form #798)Reviewer entrypoints
examples/packet_enum.rs— the environment boundary and its testsexamples/multi_packet.rs— detached-task error visibilityCargo.toml— one dev-dependency lineValidation evidence (head
0cf5a8c)This branch was restacked onto
origin/main=b194531on 2026-10-23, aftermain moved three commits including #794's toolchain bump from
nightly-2026-03-26tonightly-2026-10-22. The previous head53ee29band itslocal gate results belong to the retired toolchain and are not current
evidence.
The
weavemerge driver was disabled for the replay(
-c core.attributesFile=/dev/null), and the replay was checked against adriver-free
merge-treeresult rather than trusted.Hosted, at this head. The
build-testlane ran at this exact commit andpassed every step. It executes the repository's own targets, so its result is
per-layer evidence for the format and lint gates rather than a result inherited
from a neighbour:
make check-fmtmake lint— rustdoc with-D warnings, Clippy with-D warnings, Whitaker--allmarkdownlint-cli2over**/*.mdmake spellingmake nixiemake test-workflow-contractsmake test-feature-gatecargo llvm-cov nextest --workspace, default features and targetshttps://github.com/leynos/wireframe/actions/runs/39959682710/job/120434229692
The instrumented run executed 1207 tests, 1207 passed, 0 skipped and
reported 87.92% against the committed 87.8% baseline, so the ratchet passed.
Read from the job log, not inferred from the job conclusion.
That run is narrower than the Make targets — no
--all-targets, no--all-features, no doctests — which is why the local runs below are stillrequired.
Local. Run sequentially at this exact head with
make:Known baseline failures
None observed at this head.
Deferred work
Remaining source census findings, final lint-configuration adoption, the
testkit argument-count decision and Whitaker
--workspacecoverage.🤖 Generated with Claude Code
Summary by Sourcery
Harden workspace examples by introducing an injectable environment boundary, improving error visibility, and resolving remaining lint findings.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests:
References
harden-lint-self-named-modules-20261022)gate run and the census this layer defers to.
unsafe_code = "forbid"can apply workspace-wide #804 — isolate the unsafe allocator sounsafe_code = "forbid"can applyworkspace-wide.
/home/leynos/docs/leynos-wireframe/rust-baseline/2026-10-15/train-ledger.md🤖 Generated with Claude Code