diff --git a/.gitignore b/.gitignore index a1615a40..0f1ae4bc 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ target/ .crush .grepai/ *:Zone.Identifier +.agents/mcp/context_pack/packs/ diff --git a/docs/adr-001-multi-packet-streaming-response-api.md b/docs/adr-001-multi-packet-streaming-response-api.md index 366a4bf2..eba9b514 100644 --- a/docs/adr-001-multi-packet-streaming-response-api.md +++ b/docs/adr-001-multi-packet-streaming-response-api.md @@ -35,11 +35,12 @@ existing connection actor and protocol hook lifecycle. - Documentation describing **server-side multi-packet response handlers** MUST reflect the tuple-based API and explain the helper constructors that prepare - the channel pair (`Response::with_channel`, `Response::with_channel_and_initial`). - This requirement is scoped to handler-authoring documentation; it does not - apply to client-side streaming consumption APIs (`ResponseStream`, - `call_streaming`, `receive_streaming`, `StreamingResponseExt`, - `TypedResponseStream`), which are governed by their own documentation. + the channel pair (`Response::with_channel`, + `Response::with_channel_and_initial`). This requirement is scoped to + handler-authoring documentation; it does not apply to client-side streaming + consumption APIs (`ResponseStream`, `call_streaming`, `receive_streaming`, + `StreamingResponseExt`, `TypedResponseStream`), which are governed by their + own documentation. - The development roadmap gains concrete tasks covering helper construction, handler ergonomics, and documentation updates so the work is tracked explicitly. diff --git a/docs/contents.md b/docs/contents.md index 1a4164c0..161ef214 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -57,6 +57,8 @@ the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md - [Developers' guide](developers-guide.md) Canonical architectural vocabulary and naming invariants. +- [Frame = `Vec` inventory](frame-vec-u8-inventory.md) Inventory of frame- + and payload-shaped APIs that still rely on owned byte vectors. - [Formal verification methods](formal-verification-methods-in-wireframe.md) Recommended use of Kani, Stateright, and Verus in Wireframe. - [Refactoring guide](complexity-antipatterns-and-refactoring-strategies.md) diff --git a/docs/execplans/issue-287-inventory-trait-bounds-expecting-frame-vec-u8.md b/docs/execplans/issue-287-inventory-trait-bounds-expecting-frame-vec-u8.md new file mode 100644 index 00000000..d82b0004 --- /dev/null +++ b/docs/execplans/issue-287-inventory-trait-bounds-expecting-frame-vec-u8.md @@ -0,0 +1,209 @@ +# Inventory `Frame = Vec` trait bounds and APIs + +This ExecPlan is a living document. The sections `Constraints`, `Tolerances`, +`Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, and +`Outcomes & Retrospective` must be kept up to date as work proceeds. + +Status: COMPLETE + +No `PLANS.md` exists in this repository as of 2026-04-11. + +## Purpose / big picture + +Issue 287 supports epic 284 by producing a source-of-truth inventory of code +paths, trait bounds, impls, and APIs that currently assume `Frame = Vec` or +otherwise expose `Vec` as the de facto frame or payload representation. + +The goal is to bound migration scope without prescribing the migration itself. +The published artefact for this work is `docs/frame-vec-u8-inventory.md`. + +## Constraints + +- This issue is an inventory and planning task, not the `Bytes` migration + itself. +- The resulting document must distinguish between: + - true `Frame = Vec` constraints in traits, impls, and generic bounds; + - public APIs that expose raw payload bytes as `Vec` without literally + constraining `F::Frame = Vec`; + - internal-only usages that can likely change with lower compatibility risk; + - examples, tests, and documentation references that are illustrative but + not binding. +- Every inventory entry must cite concrete evidence: file path, symbol, and + current signature or usage shape. +- The work must reflect the current repository layout rather than historical + issue context. +- The implementation must not invent roadmap completion where no local roadmap + item exists. + +## Tolerances (exception triggers) + +- Scope: if the inventory expands beyond roughly 40 runtime-significant files, + stop and group the findings before continuing. +- Ambiguity: if more than a handful of surfaces cannot be classified as + `frame-bound`, `payload-bound`, `internal-only`, or `docs/tests-only`, stop + and define the taxonomy explicitly before proceeding. +- Epic linkage: if GitHub cannot be edited from the local environment, record + the exact document path and link text so the follow-up is trivial. + +## Risks + +- Risk: the inventory may overstate migration scope by treating all + `Vec` payload APIs as equivalent to `Frame = Vec` generic bounds. + Severity: high. Likelihood: medium. Mitigation: classify each finding by + coupling type and call out false friends explicitly. + +- Risk: stale issue context may send the investigation toward files or types + that no longer exist. Severity: medium. Likelihood: high. Mitigation: drive + the inventory from the current repository state first, and record stale + references as discoveries rather than facts. + +- Risk: documentation-only findings may drown out public API breakpoints. + Severity: medium. Likelihood: medium. Mitigation: keep separate sections for + runtime surfaces, internal-only paths, and docs/tests-only follow-up. + +## Validation and acceptance + +Acceptance is based on observable artefacts: + +- `docs/frame-vec-u8-inventory.md` exists and inventories: + - true `Frame = Vec` surfaces; + - payload-bound public APIs; + - internal-only runtime coupling; + - tests, examples, and docs that would need follow-up during epic 284. +- The inventory document includes non-prescriptive notes on generalization + paths, conceptual risks, and open questions. +- The inventory records roadmap status explicitly rather than guessing. +- Documentation quality gates pass. + +Validation commands for this documentation-only implementation: + +```sh +set -o pipefail +timeout 300 make fmt 2>&1 | tee /tmp/wireframe-fmt.log +echo "fmt exit: $?" + +set -o pipefail +timeout 300 make markdownlint 2>&1 | tee /tmp/wireframe-markdownlint.log +echo "markdownlint exit: $?" +``` + +## Context and orientation + +The current repository already shows a split between generic transport frames +and owned payload-byte APIs: + +- `src/codec.rs` defines `FrameCodec` generically over `Self::Frame`. +- `src/hooks.rs` defines `WireframeProtocol` generically over + `type Frame: FrameLike`. +- `src/app/envelope.rs`, `src/middleware.rs`, `src/client/hooks.rs`, and + `src/serializer.rs` still expose owned `Vec` payloads directly. +- `src/connection/mod.rs` uses generic frame bounds + `FrameLike + CorrelatableFrame + Packet`, which is adjacent to the migration + but is not itself a literal `Vec` constraint. + +## Plan of work + +Stage A builds a taxonomy and evidence set from current source signatures. + +Stage B inventories runtime-significant public and internal surfaces, keeping +true frame coupling separate from payload ownership APIs. + +Stage C records adjacent constraints, especially the actor/codec boundary, +without collapsing them into false `Vec` findings. + +Stage D publishes the inventory document and records epic-link and roadmap +status. + +## Concrete steps + +1. Locate the original issue-287 plan context and confirm the current + repository state. +2. Scan source files for `Vec`-shaped frame and payload surfaces. +3. Classify findings as `frame-bound`, `payload-bound`, `internal-only`, or + `docs/tests-only`. +4. Publish `docs/frame-vec-u8-inventory.md`. +5. Update the docs index and record roadmap and epic-link follow-up notes. +6. Run documentation gates. + +## Idempotence and recovery + +The source scan, classification, and documentation steps are all re-runnable. +If a first pass over-collects findings, keep the evidence and reclassify it +rather than deleting it. If follow-up work later changes the coupling shape, +publish a superseding inventory rather than silently mutating historical +conclusions. + +## Progress + +- [x] (2026-04-10) Original issue-287 ExecPlan drafted in commit `c0cbc75`. +- [x] (2026-04-11) Recovered the missing ExecPlan from repository history + because the file was absent from the current worktree. +- [x] (2026-04-11) Re-scanned current runtime code and classified findings by + coupling type. +- [x] (2026-04-11) Published `docs/frame-vec-u8-inventory.md`. +- [x] (2026-04-11) Added the inventory to `docs/contents.md`. +- [x] (2026-04-11) Recorded roadmap status and prepared epic-link text in the + inventory document. +- [x] (2026-04-11) Ran documentation quality gates: + `make fmt` and `make markdownlint`. + +## Surprises & Discoveries + +- Observation: the exact issue-287 ExecPlan file was not present in the + current worktree. Evidence: direct file lookup failed, but `git log --all` + showed commit `c0cbc75` creating + `docs/execplans/issue-287-inventory-trait-bounds-expecting-frame-vec-u8.md`. + Impact: the implementation had to recover planning context from history + before proceeding. + +- Observation: the runtime no longer hard-codes `F::Frame = Vec` at the + codec or protocol trait layer. Evidence: `src/codec.rs` and `src/hooks.rs`. + Impact: the main migration scope is payload ownership and mutability, not a + blanket frame-type constraint. + +- Observation: the strongest remaining coupling is public payload-oriented API + shape. Evidence: `src/app/envelope.rs`, `src/middleware.rs`, + `src/client/hooks.rs`, and `src/serializer.rs`. Impact: epic 284 should + separate transport-frame work from payload-API work. + +- Observation: no matching roadmap item for this inventory work exists in the + local docs set. Evidence: repository-wide roadmap search on 2026-04-11. + Impact: nothing was marked done. + +## Decision Log + +- Decision: publish the final inventory as + `docs/frame-vec-u8-inventory.md`. Rationale: the path is explicit, stable, + and easy to link from epic 284. Date/Author: 2026-04-11 / Codex. + +- Decision: restore the missing issue-287 ExecPlan path in the worktree. + Rationale: the user explicitly referenced this path, and the historical plan + existed in commit history. Restoring it keeps the documentation trail intact. + Date/Author: 2026-04-11 / Codex. + +- Decision: classify findings by coupling type instead of flattening them into + a grep dump. Rationale: migration planning needs scope-ranked evidence, not + only a list of matches. Date/Author: 2026-04-11 / Codex. + +## Outcomes & Retrospective + +Completed implementation outcomes: + +- Published `docs/frame-vec-u8-inventory.md` as the issue-287 source of truth. +- Distinguished true `Frame = Vec` surfaces from payload-bound APIs, + internal-only runtime coupling, and docs/tests-only follow-up. +- Recorded the actor/codec boundary as adjacent context rather than a false + `Vec` finding. +- Added the inventory to `docs/contents.md` for discoverability. +- Recorded that no matching roadmap item was found locally and prepared + epic-link text for external follow-up. + +Validation outcomes: + +- `make fmt` passed. +- `make markdownlint` passed. + +## Revision note + +This file was restored on 2026-04-11 from commit `c0cbc75` and updated to +reflect completion of the documentation work in the current worktree. diff --git a/docs/frame-vec-u8-inventory.md b/docs/frame-vec-u8-inventory.md new file mode 100644 index 00000000..55ca79a5 --- /dev/null +++ b/docs/frame-vec-u8-inventory.md @@ -0,0 +1,393 @@ +# Inventory of `Frame = Vec` trait bounds and APIs + +This document records the current code paths, trait bounds, implementations, +and documentation that still assume `Frame = Vec` or otherwise expose owned +`Vec` values as the de facto frame or payload representation. Issue 287 +uses this inventory to support epic 284 by bounding the migration scope without +prescribing a single implementation sequence. Issue 286 extends that work with +a middleware-specific analysis after PR #283 reduced frame-processing +allocations.[^adr-004][^issue-286] + +## Scope and taxonomy + +This inventory distinguishes four kinds of coupling: + +| Category | Meaning | Migration sensitivity | +| ----------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- | +| `frame-bound` | A trait, impl, or test uses `Vec` as a concrete frame type. | High when the transport frame type changes. | +| `payload-bound` | A public application programming interface (API) exposes owned payload bytes as `Vec` without literally fixing `F::Frame = Vec`. | High for API compatibility, lower for codec plumbing. | +| `internal-only` | Runtime code carries `Vec` internally without exposing it as a stable API. | Medium; usually easier to change behind shims. | +| `docs/tests-only` | Examples, tests, and design text that still teach `Vec`-shaped framing. | Low runtime risk, but high drift risk. | + +Two important boundaries are out of scope for this inventory: + +- General `Vec` usage that is clearly about message bodies, fragment + payloads, or unrelated buffering rather than transport frames or payload + ownership hand-offs. +- Selecting `Bytes`, `BytesMut`, or another abstraction as the replacement. + This document records coupling and trade-offs only. + +## Executive summary + +The codebase no longer hard-codes `F::Frame = Vec` at the top-level codec +or protocol abstraction. `FrameCodec` is generic over `Self::Frame`, and +`WireframeProtocol` is generic over `type Frame: FrameLike`.[^codec][^hooks] +The main remaining migration scope is instead concentrated in public +payload-owned APIs: + +- `PacketParts`, `Envelope`, `ServiceRequest`, and `ServiceResponse` all own + `Vec` payloads or frames. +- Client request hooks and preamble replay APIs still expose mutable or owned + `Vec` buffers. +- `Serializer::serialize` still returns `Vec`, which keeps outbound + serialization byte-owned even when codecs use `Bytes`. +- Epic 284 should cover both transport-frame substitution and public + payload-owned APIs, but those should be tracked as separate workstreams under + one umbrella so consuming developers do not have to write avoidable adapter + boilerplate. + +The main adjacent constraint is not a literal `Vec` bound. The +`ConnectionActor` requires `F: FrameLike + CorrelatableFrame + Packet`, which +the default codec frame type (`Bytes`) does not satisfy.[^unified-path] This is +why the current server path routes actor output through `Envelope` and the +codec driver rather than sending transport frames through the actor directly. + +## Inventory summary + +| Category | Representative surfaces | Primary files | +| ----------------- | --------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | +| `frame-bound` | `CorrelatableFrame for Vec`, test-only `Packet for Vec` | `src/correlation.rs`, `src/connection/test_support.rs` | +| `payload-bound` | Packets, middleware, client request hooks, serializer output | `src/app/envelope.rs`, `src/middleware.rs`, `src/client/hooks.rs`, `src/serializer.rs` | +| `internal-only` | DLQ sender, response forwarding, preamble replay | `src/app/builder/core.rs`, `src/app/frame_handling/response.rs`, `src/rewind_stream.rs` | +| `docs/tests-only` | Protocol tests, guide examples, design diagrams | `tests/wireframe_protocol.rs`, `docs/users-guide.md`, `docs/asynchronous-outbound-messaging-design.md` | + +## Confirmed runtime surfaces + +### True `frame-bound` surfaces + +- `src/correlation.rs` implements `CorrelatableFrame` for `Vec`. + This makes raw byte vectors participate directly in generic frame plumbing, + even though the implementation is a no-op correlation carrier. +- `src/connection/test_support.rs` implements `Packet` for `Vec` in test + support. This is not production API, but it proves the connection actor stack + still treats `Vec` as a first-class frame type in its generic harnesses. + +These are the only confirmed runtime code paths where `Vec` is promoted to +an actual frame type today. The public codec and protocol traits themselves do +not require it. + +### Public `payload-bound` surfaces + +#### Packets and envelopes + +- `src/app/envelope.rs` defines `PacketParts` with + `payload: Vec`, `PacketParts::new(..., payload: Vec)`, and + `PacketParts::into_payload(self) -> Vec`. +- The same file defines `Envelope` with `payload: Vec` and + `Envelope::new(id, correlation_id, payload: Vec)`. +- `Packet` itself is payload-oriented through + `fn into_parts(self) -> PacketParts` and + `fn from_parts(parts: PacketParts) -> Self`. + +These are not transport-frame constraints, but they are high-sensitivity public +APIs because routing, middleware reconstruction, and packet rebuilding all +depend on owned payload bytes. + +#### Middleware request and response wrappers + +- `src/middleware.rs` fixes `ServiceRequest` to + `inner: FrameContainer>`. +- `ServiceRequest::new(frame: Vec, correlation_id)` and + `ServiceRequest::frame_mut(&mut self) -> &mut Vec` expose owned, mutable + bytes directly to middleware. +- `ServiceResponse` mirrors the same contract with + `inner: FrameContainer>`, + `ServiceResponse::new(frame: Vec, ...)`, + `frame_mut(&mut self) -> &mut Vec`, and `into_inner(self) -> Vec`. + +This is one of the clearest byte-ownership APIs in the crate. Any migration +that changes the type here becomes a user-visible middleware API change. + +#### Middleware boundary analysis (issue 286) + +Issue 286 asks for a focused read on the middleware layer, including its data +flow, impacted integration points, and conceptual adaptation strategies after +PR #283 moved more of the codec path toward `Bytes`.[^issue-286] + +##### Middleware data flow + +The middleware layer currently sits entirely inside an owned-`Vec` segment +of the request/response pipeline: + +1. `src/app/inbound_handler.rs` builds or reuses the middleware chain through + `WireframeApp::build_chains()`, wrapping each route handler inside + `HandlerService`. +2. `src/app/frame_handling/decode.rs` and `src/app/inbound_handler.rs` decode + an inbound codec frame into an `Envelope`. +3. `src/app/frame_handling/response.rs` crosses the middleware boundary by + calling `ServiceRequest::new(env.payload, env.correlation_id)`. +4. `src/middleware.rs` exposes that payload to middleware as + `ServiceRequest`, with `frame()`, `frame_mut()`, `set_correlation_id()`, and + `into_inner()`. +5. The terminal bridge in `src/middleware.rs` (`RouteService::call`) converts + the request back into a packet with + `E::from_parts(PacketParts::new(self.id, req.correlation_id(), req.into_inner()))`, + invokes the registered handler, then returns + `ServiceResponse::new(payload, correlation_id)`. +6. `src/app/frame_handling/response.rs` crosses back out of middleware by + rebuilding `PacketParts` and `Envelope` from `resp.into_inner()`. +7. `src/app/codec_driver.rs` serializes that envelope to bytes via + `Serializer::serialize`, converts the `Vec` into `Bytes`, and finally + hands it to `FrameCodec::wrap_payload`. + +The practical consequence is that middleware currently forms a hard +`Vec`-owned island between generic codec frames on the wire and the +`Bytes`-friendly codec boundary on the way back out. + +##### Middleware integration points affected by a frame-type change + +- `src/middleware.rs` is the primary public API surface: + `ServiceRequest`, `ServiceResponse`, `Service`, `Transform`, `Next`, + `from_fn`, `HandlerService::from_service`, and the `frame_mut()` / + `into_inner()` editing model all currently depend on `Vec`. +- `src/app/frame_handling/response.rs` is the request/response bridge into and + out of middleware. Any type change has to preserve correlation handling and + packet reconstruction here. +- `src/app/inbound_handler.rs` owns middleware-chain construction and therefore + any staging or compatibility story for mixed old/new middleware types. +- `tests/middleware.rs` and `tests/middleware_order.rs` demonstrate the current + mutation semantics directly through `push`, `clear`, and `extend_from_slice` + on `frame_mut()`. +- `docs/users-guide.md` teaches middleware authors to decode requests from + `req.frame()` and rewrite replies through `response.frame_mut()`. +- `wireframe_testing/src/helpers/drive.rs` is an adjacent test surface: it + still feeds raw `Vec` wire frames into apps, which matters for end-to-end + middleware tests even though it is not itself the middleware API. + +##### Middleware-specific risks + +- Blast radius is high because every request passes through the middleware + wrappers, and external middleware implementations are written against + `Vec` constructors and mutation helpers today. +- Switching directly to immutable `Bytes` would break the current editing model + around `frame_mut()`. +- Switching directly to another mutable buffer type could still impose + widespread boilerplate if consumers have to manually convert between + `Vec`, `Bytes`, and `BytesMut`. + +##### Conceptual adaptation strategies + +The following strategies are intentionally non-prescriptive, but they bound the +design space: + +- Track middleware payload APIs as a separate epic-284 workstream from + transport-frame substitution. The two interact, but they do not need to land + in a single breaking change. +- Prefer developer ergonomics over exposing raw buffer taxonomy. Middleware + authors should not have to hand-write repetitive conversions between byte + container types to achieve routine inspection and mutation. +- If middleware internals move toward a `Bytes`-compatible representation, keep + mutation opt-in and edit-oriented. A copy-on-write or edit-on-demand surface + is conceptually safer than forcing every middleware author to manage shared + byte ownership directly. +- Preserve the bounded special case for client preamble leftovers. That path is + one-shot and replay-only, so it does not need to be pulled into the + middleware or transport-frame redesign unless the broader public byte APIs + standardize on a new shared abstraction. + +#### Client request hooks + +- `src/client/hooks.rs` exports + `BeforeSendHook = Arc) + Send + Sync>`. +- `src/client/builder/request_hooks.rs` exposes the same shape through + `before_send(...) where F: Fn(&mut Vec) + Send + Sync + 'static`. +- `src/client/messaging.rs` applies these hooks via + `invoke_before_send_hooks(&self, bytes: &mut Vec)`. + +This is a payload-bound API rather than a literal `Frame = Vec` bound, but +it is still a compatibility surface because the client promises mutable access +to serialized outbound bytes before transport framing. + +#### Client preamble leftovers + +- `src/client/mod.rs` exports `ClientPreambleSuccessHandler` as a callback + returning `io::Result>`. +- `src/client/preamble_exchange.rs` stores and executes that callback through + `perform_preamble_exchange(...) -> Result, ClientError>` and + `run_preamble_exchange(...) -> Result, ClientError>`. +- `src/rewind_stream.rs` then replays those leftover bytes through + `RewindStream::new(leftover: Vec, inner)`. + +This is another public owned-bytes contract. It is adjacent to transport +framing because the leftover bytes are replayed before framed communication +starts, but the surface is about preamble buffering rather than `F::Frame`. + +#### Serializer output + +- `src/serializer.rs` defines + `Serializer::serialize(&self, value: &M) -> Result, ...>`. +- `BincodeSerializer` preserves that same return type. + +This does not force transport frames to be `Vec`, because codecs can still +wrap those bytes into `Bytes` or protocol-native frame structs. It does, +however, keep outbound message serialization owned and mutable at the boundary +where several other APIs also expect `Vec`. + +### `internal-only` runtime surfaces + +- `src/app/builder/core.rs` stores `push_dlq` as a dead-letter queue (DLQ) + sender: `Option>>`. +- `src/app/builder/config.rs` exposes that internal channel through + `with_push_dlq(self, dlq: mpsc::Sender>) -> Self`. +- `src/app/frame_handling/response.rs` shows the current payload flow + explicitly: `ServiceRequest::new(env.payload, env.correlation_id)` moves the + inbound envelope payload into middleware, and + `PacketParts::new(env.id, resp.correlation_id(), resp.into_inner())` + reconstructs an outbound envelope from `Vec`. + +These are lower-risk than the public packet and middleware APIs, but they are +still part of the migration blast radius because they connect the public byte +contracts to the internal send path. + +## Adjacent constraints that matter but do not name `Vec` + +These surfaces are essential migration context even though they are not +themselves `Frame = Vec` bindings: + +- `src/connection/mod.rs` requires + `F: FrameLike + CorrelatableFrame + Packet` for the `ConnectionActor`. +- `src/app/builder/protocol.rs` stores protocols as + `WireframeProtocol`. +- `src/codec.rs` defines the default `LengthDelimitedFrameCodec` with + `Bytes` frames, not `Vec`. + +Taken together, these show that the main frame-level tension is between generic +actor requirements and codec frame capabilities, not between the actor and +`Vec` specifically. The current unified server path therefore routes actor +output through `Envelope` and lets the codec driver perform the final +`wrap_payload` step.[^unified-path] + +For epic 284, this means transport-frame generalization and payload-API +generalization should be tracked separately. A code path can be generic over +`F::Frame` while still exposing `Vec` in middleware, packets, or hooks. + +## Tests, examples, and documentation that still teach `Vec` + +### Tests and harnesses + +- `tests/wireframe_protocol.rs` defines a test codec with `type Frame = Vec` + and a protocol implementation with `type Frame = Vec`. +- `src/connection/test_support.rs` uses `Packet for Vec` to drive actor + tests. + +These are not runtime blockers, but they confirm that `Vec` remains the +default teaching and testing shape for protocol hooks. + +### User and design documentation + +- `docs/users-guide.md` still includes protocol examples with + `type Frame = Vec` and client hook examples using `&mut Vec`. +- `docs/asynchronous-outbound-messaging-design.md` still contains diagrams and + narrative that show `WireframeProtocol, ProtocolError = ()>`. +- `docs/wireframe-client-design.md` describes `before_send` as operating on + `&mut Vec`. + +These documents need follow-up whenever the public byte-oriented APIs change, +otherwise the written guidance will drift from the implementation. + +## What is already generic today + +Several prominent abstractions already distinguish transport frames from owned +payload bytes: + +- `FrameCodec` is generic over `Self::Frame` and already supports `Bytes` and + protocol-native frame structs.[^codec] +- `WireframeProtocol` is generic over `type Frame: FrameLike`, so the protocol + trait itself is not coupled to `Vec`.[^hooks] +- `ProtocolHooks` is also generic over `F`. + +This means epic 284 does not start from a codebase that hard-codes `Vec` +everywhere. The remaining work is concentrated in the payload-owned surfaces +listed above and in the actor/codec boundary described by the unified +codec-path work.[^unified-path] + +## Generalization paths and conceptual risks + +The inventory suggests several non-prescriptive workstreams: + +- Separate transport-frame work from payload-ownership work. Changing + `FrameCodec::Frame` does not automatically solve middleware, packet, + serializer, or client-hook APIs that still promise owned `Vec` values. +- Keep the actor-boundary problem separate from the `Vec` inventory. The + `ConnectionActor` currently wants `Packet + CorrelatableFrame`; changing only + byte container types will not remove that constraint. +- Plan for a long-term outbound boundary that is `Bytes`-compatible rather than + `Vec`-centric, while retaining `Vec` compatibility adapters for as + long as public mutation hooks still require owned mutable bytes. +- Remove `CorrelatableFrame for Vec` from the core surface once raw + `Vec` stops being a documented or production frame shape. If a bridge is + still useful, keep it in docs, `test-support`, or a feature-gated + compatibility shim. +- Keep client preamble leftovers as owned `Vec` values for now. Revisit + that path only if the broader public byte APIs converge on a shared buffer + abstraction. + +The main conceptual risk is overstating the migration scope by treating every +`Vec` as a frame problem. In practice, most runtime-sensitive surfaces are +about payload ownership and mutability, while true frame-level `Vec` +coupling is now limited. + +## Resolved direction for epic 284 + +The current direction is now explicit: + +- Epic 284 covers both transport-frame substitution and public payload-owned + APIs such as `PacketParts`, middleware, and client hooks. +- Those changes should be tracked as separate workstreams under the same + umbrella so the migration can preserve developer ergonomics and minimize + boilerplate. +- The long-term outbound boundary should be `Bytes`-compatible rather than + `Vec`-centric, with `Vec` retained only as a compatibility adapter + while mutable byte-edit hooks still require it. +- `CorrelatableFrame for Vec` should leave the core surface once raw + `Vec` stops serving as a documented or production frame shape. +- Client preamble leftovers remain a separate compatibility surface and should + stay as owned `Vec` for now. + +## Coordination notes + +No matching roadmap item for this inventory work was found in the local docs +set on 2026-04-11, so nothing was marked done. If epic 284 is updated outside +the repository, the prepared link text and next-step summary are: + +```plaintext +Add inventory reference: docs/frame-vec-u8-inventory.md +Link label: Frame = Vec inventory + +Epic 284 workstreams: +1. Transport-frame substitution and actor/codec boundary work. +2. Public payload-owned API work: PacketParts, Envelope, middleware, and + client byte hooks, with minimal consumer boilerplate. +3. Compatibility cleanup: move Vec-only bridges such as + CorrelatableFrame for Vec out of the core surface once no longer needed. +4. Deferred: keep client preamble leftovers on Vec until broader public + byte APIs justify standardization. +``` + +[^adr-004]: See + [Architecture Decision Record (ADR) 004](adr-004-pluggable-protocol-codecs.md) + for the generic codec decision and the current `Bytes` default frame type. +[^issue-286]: Middleware follow-up requested by + [@leynos](https://github.com/leynos), with context from + [issue #286](https://github.com/leynos/wireframe/issues/286), + [PR #283](https://github.com/leynos/wireframe/pull/283), and the linked + [comment](https://github.com/leynos/wireframe/pull/283#issuecomment-3167997856). +[^codec]: See + [Pluggable protocol codecs](execplans/pluggable-protocol-codecs.md) + and `src/codec.rs`. +[^hooks]: See `src/hooks.rs` and the hook overview in + [users-guide.md](users-guide.md). +[^unified-path]: See + [docs/execplans/9-3-1-fragment-adapter-trait.md](./execplans/9-3-1-fragment-adapter-trait.md) + for the actor/codec-driver boundary and why `Bytes` does not flow through + `ConnectionActor` directly today. diff --git a/docs/v0-2-0-to-v0-3-0-migration-guide.md b/docs/v0-2-0-to-v0-3-0-migration-guide.md index e73e4cef..fc8fd7b8 100644 --- a/docs/v0-2-0-to-v0-3-0-migration-guide.md +++ b/docs/v0-2-0-to-v0-3-0-migration-guide.md @@ -200,8 +200,9 @@ wireframe = { version = "0.3.0", features = ["testkit"] } ## Unified error surface -`wireframe` now exposes one canonical error type: `wireframe::WireframeError`, -defined in `wireframe::error`. The two previous error types are gone. +`wireframe` now exposes one canonical error type: +`wireframe::WireframeError`, defined in `wireframe::error`. The two previous +error types are gone. - `wireframe::app::error::WireframeError` is removed. - `wireframe::response::WireframeError` is removed. @@ -236,21 +237,21 @@ reference the module directly rather than the root shorthand. The most common moves: -| Removed root path | New path | -| --- | --- | -| `wireframe::BincodeSerializer`, `wireframe::Serializer` | `wireframe::serializer::{BincodeSerializer, Serializer}` | -| `wireframe::ConnectionActor` | `wireframe::connection::ConnectionActor` | -| `wireframe::CorrelatableFrame` | `wireframe::correlation::CorrelatableFrame` | +| Removed root path | New path | +| ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------- | +| `wireframe::BincodeSerializer`, `wireframe::Serializer` | `wireframe::serializer::{BincodeSerializer, Serializer}` | +| `wireframe::ConnectionActor` | `wireframe::connection::ConnectionActor` | +| `wireframe::CorrelatableFrame` | `wireframe::correlation::CorrelatableFrame` | | `wireframe::ConnectionContext`, `wireframe::ProtocolHooks`, `wireframe::WireframeProtocol` | `wireframe::hooks::{ConnectionContext, ProtocolHooks, WireframeProtocol}` | -| `wireframe::ClientCodecConfig`, `wireframe::ClientError`, `wireframe::WireframeClient` | `wireframe::client::{...}` | -| `wireframe::CodecError`, `wireframe::DefaultRecoveryPolicy`, `wireframe::RecoveryConfig` | `wireframe::codec::{...}` | -| `wireframe::FrameStream`, `wireframe::Response` | `wireframe::response::{FrameStream, Response}` | -| `wireframe::ConnectionId`, `wireframe::SessionRegistry` | `wireframe::session::{ConnectionId, SessionRegistry}` | -| `wireframe::RequestParts`, `wireframe::RequestBodyStream` | `wireframe::request::{RequestParts, RequestBodyStream}` | +| `wireframe::ClientCodecConfig`, `wireframe::ClientError`, `wireframe::WireframeClient` | `wireframe::client::{...}` | +| `wireframe::CodecError`, `wireframe::DefaultRecoveryPolicy`, `wireframe::RecoveryConfig` | `wireframe::codec::{...}` | +| `wireframe::FrameStream`, `wireframe::Response` | `wireframe::response::{FrameStream, Response}` | +| `wireframe::ConnectionId`, `wireframe::SessionRegistry` | `wireframe::session::{ConnectionId, SessionRegistry}` | +| `wireframe::RequestParts`, `wireframe::RequestBodyStream` | `wireframe::request::{RequestParts, RequestBodyStream}` | The full list of moved types is in the -[v0.1.0 → v0.2.0 migration guide](v0-1-0-to-v0-2-0-migration-guide.md). -That list now takes effect. +[v0.1.0 → v0.2.0 migration guide](v0-1-0-to-v0-2-0-migration-guide.md). That +list now takes effect. `wireframe::prelude::*` provides an optional convenience import for high-frequency types. It is intentionally small; prefer direct module imports @@ -336,14 +337,14 @@ impl EncodeWith for MyMessage { } ``` -A `SerdeSerializerBridge` trait (behind the `serializer-serde` feature) and -the `SerdeMessage` wrapper provide a path for Serde-derived types without +A `SerdeSerializerBridge` trait (behind the `serializer-serde` feature) and the +`SerdeMessage` wrapper provide a path for Serde-derived types without implementing `bincode::Encode`. ## WireframeApp construction -`WireframeApp::new()` now returns `Result` for forward compatibility. -The call currently always succeeds, but the return type must be handled. +`WireframeApp::new()` now returns `Result` for forward compatibility. The +call currently always succeeds, but the return type must be handled. ```rust // Before @@ -611,7 +612,8 @@ partial frames, fragments, and slow I/O, plus `TestSerializer`, `TestResult`, `TestError`, and assembly snapshot assertion helpers. The `wireframe_testing` companion crate provides `WireframePair` (real loopback server–client pair), `ObservabilityHandle` (log + metrics capture with atomic snapshot semantics), -`Labels`, and `HotlineFixtures` (codec regression fixtures and `new_test_codec`).* +`Labels`, and `HotlineFixtures` (codec regression fixtures and +`new_test_codec`).* `wireframe::testkit` (behind the `testkit` Cargo feature) provides optional test utilities for downstream protocol crates. The module is not available in @@ -627,8 +629,8 @@ Capabilities include: real network delay. - Assembly assertion helpers (`assert_message_assembly_completed`, `assert_fragment_reassembly_error`, and the full `MessageAssemblySnapshot` / - `FragmentReassemblySnapshot` families) for verifying assembly and - reassembly outcomes without panicking. + `FragmentReassemblySnapshot` families) for verifying assembly and reassembly + outcomes without panicking. - `TestSerializer` – a minimal serializer for use in tests. - `TestResult` and `TestError` – ergonomic result types for test functions. @@ -700,8 +702,8 @@ assert_eq!( ``` `obs_handle()` is an `rstest` fixture that constructs a handle directly. -`Labels` provides a builder for label pairs used with `ObservabilityHandle:: -counter`. +`Labels` provides a builder for label pairs used with +`ObservabilityHandle::counter`. The handle's `snapshot()` method drains counters atomically. Query after `snapshot()`; earlier values are not retained. @@ -885,14 +887,15 @@ yielding a `SendStreamingOutcome`.* request, and returns a `ResponseStream` that yields typed frames until the server sends a stream terminator. - `receive_streaming(correlation_id)` – the lower-level variant for callers - that have already sent the request via `send` or `send_envelope` and hold - the correlation identifier. Returns a `ResponseStream` that validates every + that have already sent the request via `send` or `send_envelope` and hold the + correlation identifier. Returns a `ResponseStream` that validates every inbound frame against that identifier. -`ResponseStream` implements `futures::Stream` with `Item = Result`. It holds an exclusive borrow of the client for the duration of -the stream, preventing concurrent sends. The terminator frame is consumed -internally and the stream returns `None` once it arrives. +`ResponseStream` implements `futures::Stream` with +`Item = Result`. It holds an exclusive borrow of the +client for the duration of the stream, preventing concurrent sends. The +terminator frame is consumed internally and the stream returns `None` once it +arrives. ```rust use std::net::SocketAddr; @@ -945,16 +948,16 @@ sequenceDiagram ``` *Sequence diagram: `call_streaming` sends the request and returns a -`ResponseStream` that holds an exclusive borrow of the client. Each `try_next()` -call polls the client for the next server frame; the loop continues until the -server sends a terminator, at which point the stream returns `None`. Dropping -the `ResponseStream` releases the borrow, making the client available for -further calls.* +`ResponseStream` that holds an exclusive borrow of the client. Each +`try_next()` call polls the client for the next server frame; the loop +continues until the server sends a terminator, at which point the stream +returns `None`. Dropping the `ResponseStream` releases the borrow, making the +client available for further calls.* `StreamingResponseExt` is a trait on any `Stream` of response frames. It -provides `typed_with(mapper)`, which produces a `TypedResponseStream` that translates raw frames into domain values and skips control -frames where the mapper returns `Ok(None)`. +provides `typed_with(mapper)`, which produces a +`TypedResponseStream` that translates raw frames into +domain values and skips control frames where the mapper returns `Ok(None)`. ```rust use wireframe::client::StreamingResponseExt; @@ -973,8 +976,8 @@ let typed_stream = raw_stream.typed_with(|frame| { `WireframeClient::send_streaming` sends a large body as a sequence of protocol-framed chunks, reading from any `AsyncRead` source. -`SendStreamingConfig` controls chunk sizing and timeout. When chunk size is -not set, it is derived from `max_frame_length - frame_header.len()`. +`SendStreamingConfig` controls chunk sizing and timeout. When chunk size is not +set, it is derived from `max_frame_length - frame_header.len()`. ```rust use std::time::Duration; @@ -1086,11 +1089,11 @@ classDiagram ClientPoolConfig --> PoolFairnessPolicy ``` -*Class diagram: `WireframeClientBuilder` creates a `WireframeClientPool` using a -`ClientPoolConfig` (which selects a `PoolFairnessPolicy`). The pool vends -`PoolHandle` instances; each handle yields a `PooledClientLease` on `acquire()`. -`PooledClientLease` derefs to `WireframeClient`, exposing `call`, `send`, and -`receive` for individual requests.* +*Class diagram: `WireframeClientBuilder` creates a `WireframeClientPool` using +a `ClientPoolConfig` (which selects a `PoolFairnessPolicy`). The pool vends +`PoolHandle` instances; each handle yields a `PooledClientLease` on +`acquire()`. `PooledClientLease` derefs to `WireframeClient`, exposing `call`, +`send`, and `receive` for individual requests.* Sequence diagram showing the connection pool usage flow: building the pool, acquiring a PoolHandle, checking out a PooledClientLease, making a request, and @@ -1171,9 +1174,9 @@ let client = WireframeClientBuilder::new() `TracingConfig` controls which client operations emit tracing spans, at what level, and whether per-operation elapsed-time events are recorded. -The default configuration emits `INFO` spans for lifecycle operations (`connect`, -`close`) and `DEBUG` spans for data operations (`send`, `receive`, `call`, -`call_streaming`). Timing is disabled by default. +The default configuration emits `INFO` spans for lifecycle operations +(`connect`, `close`) and `DEBUG` spans for data operations (`send`, `receive`, +`call`, `call_streaming`). Timing is disabled by default. ```rust use tracing::Level;