Publish Frame Vec<u8> inventory docs and recover ExecPlan#522
Merged
Conversation
…nd APIs - Published comprehensive inventory documenting all code paths, trait bounds, implementations, and public/internal APIs that still assume or expose `Frame = Vec<u8>`, supporting epic 284 migration planning. - Distinguished coupling categories: true frame-bound, payload-bound public API, internal-only runtime surfaces, and docs/tests-only references. - Updated documentation index to include new frame-vec-u8-inventory.md. - Cleaned up related multi-packet streaming response API docs. - Revised migration guide to improve formatting and clarify moved types. - Ensured documentation quality gates pass (formatting and markdown linting). This addition bounds the migration scope clearly without prescribing any migration sequence, aiding subsequent generalization of transport frame and payload ownership abstractions. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Docs-only change: Frame = Vec inventory and Issue 287 ExecPlanThis pull request restores the Issue 287 ExecPlan and publishes a repository inventory of APIs and trait bounds that assume Frame = Vec, supporting epic 284 by bounding migration surface for transport-frame generalisation. New documentation
Supporting changes
Validation
WalkthroughDocument the remaining uses of Frame = Vec with a new inventory and execplan, update the docs contents and migration guide formatting, and add a gitignore pattern to ignore agent context_pack packs. Changes
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Reviewer's GuideDocs-only PR that restores and publishes the Issue 287 ExecPlan and a detailed Frame = Vec inventory document, wires them into the docs index, and makes small wording/formatting improvements to existing ADR and migration docs for clarity and readability. Sequence diagram for middleware request/response flow in the Frame Vec_u8 inventorysequenceDiagram
participant CodecDriver
participant FrameCodec
participant InboundHandler
participant Envelope
participant Middleware as MiddlewareLayer
participant Handler as RouteHandler
participant Serializer
CodecDriver->>FrameCodec: decode(raw_bytes)
FrameCodec-->>CodecDriver: frame
CodecDriver->>InboundHandler: frame
InboundHandler->>Envelope: build_from_frame(frame)
Envelope-->>InboundHandler: env
InboundHandler->>Middleware: ServiceRequest::new(env.payload, env.correlation_id)
Middleware-->>Middleware: frame_mut()/frame() edits Vec_u8
Middleware->>RouteHandler: call(ServiceRequest)
RouteHandler-->>Middleware: ServiceResponse::new(payload, correlation_id)
Middleware-->>InboundHandler: ServiceResponse
InboundHandler->>Envelope: PacketParts::new(env.id, resp.correlation_id(), resp.into_inner())
Envelope-->>InboundHandler: outbound_env
InboundHandler->>Serializer: serialize(outbound_env)
Serializer-->>InboundHandler: Vec_u8
InboundHandler->>FrameCodec: wrap_payload(Bytes::from(Vec_u8))
FrameCodec-->>CodecDriver: outbound_frame
CodecDriver-->>CodecDriver: send(outbound_frame)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…move lock file 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.
leynos
changed the title
…ysis Add detailed inventory and analysis of middleware payload ownership APIs related to Vec<u8> frames in docs/frame-vec-u8-inventory.md. Document middleware data flow, integration points, risks, and conceptual adaptation strategies following issue 286 and PR 283. Clarify distinct epic 284 workstreams covering transport-frame substitution and public payload APIs to guide migration toward Bytes-compatible boundaries while maintaining developer ergonomics and minimal boilerplate. Also update related coordination notes to reflect resolved direction for epic 284. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This was
linked to
issues
Apr 12, 2026
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docs/frame-vec-u8-inventory.md" line_range="19" />
<code_context>
+| ----------------- | ---------------------------------------------------------------------------------------------------- | ----------------------------------------------------- |
+| `frame-bound` | A trait, impl, or test uses `Vec<u8>` as a concrete frame type. | High when the transport frame type changes. |
+| `payload-bound` | A public API exposes owned payload bytes as `Vec<u8>` without literally fixing `F::Frame = Vec<u8>`. | High for API compatibility, lower for codec plumbing. |
+| `internal-only` | Runtime code carries `Vec<u8>` 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<u8>`-shaped framing. | Low runtime risk, but high drift risk. |
+
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “API” is used without expansion; per the style guide uncommon acronyms should be defined on first use.
Consider expanding “API” on its first occurrence, for example “application programming interface (API)”, to comply with the requirement to define acronyms on first use.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Define uncommon acronyms on first use.
</details>
</issue_to_address>
### Comment 2
<location path="docs/frame-vec-u8-inventory.md" line_range="238" />
<code_context>
+
+### `internal-only` runtime surfaces
+
+- `src/app/builder/core.rs` stores `push_dlq` as
+ `Option<mpsc::Sender<Vec<u8>>>`.
+- `src/app/builder/config.rs` exposes that internal channel through
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “dlq” (in `push_dlq`) is used without being expanded; it should be defined as “dead-letter queue (DLQ)” on first use.
Since DLQ is not a universally known term, please expand it at its first mention, for example “dead-letter queue (DLQ)”, and then you can keep using “DLQ” elsewhere in the docs.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Define uncommon acronyms on first use.
</details>
</issue_to_address>
### Comment 3
<location path="docs/frame-vec-u8-inventory.md" line_range="377" />
<code_context>
+ byte APIs justify standardization.
+```
+
+[^adr-004]: See [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
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym “ADR” is introduced without expansion; it should be defined (e.g. “Architecture Decision Record (ADR)”) on first use.
Please expand ADR at its first mention, for example “Architecture Decision Record (ADR) 004”, to satisfy the requirement to define acronyms on first use.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Define uncommon acronyms on first use.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/frame-vec-u8-inventory.md`:
- Around line 389-391: The footnote reference [^unified-path] uses an
absolute-ish href that doesn't resolve; update the link target in
docs/frame-vec-u8-inventory.md (the footnote block that currently points to
docs/execplans/9-3-1-fragment-adapter-trait.md) to a relative path from this
file such as ./execplans/9-3-1-fragment-adapter-trait.md so the link resolves
correctly; edit the footnote line that contains the href to replace the old path
with the new relative path.
In `@docs/v0-2-0-to-v0-3-0-migration-guide.md`:
- Around line 705-707: Fix the malformed method reference by removing the stray
space so the docs reference the symbol correctly: change the text from
"ObservabilityHandle:: counter" to "ObservabilityHandle::counter" where `Labels`
and `ObservabilityHandle` are mentioned so the method symbol
`ObservabilityHandle::counter` is resolvable.
- Around line 894-897: The doc incorrectly shows the generic type `P` for
ResponseStream's stream item; update the text to use the concrete frame type
used across the guide (e.g. change `Item = Result<P, ClientError>` to `Item =
Result<Frame, ClientError>`) so the description of ResponseStream,
futures::Stream, and its ownership/termination behavior matches the rest of the
docs and the actual type returned by ResponseStream.
🪄 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: 4f3b65dc-0a89-49a2-b446-e7e88b3add57
📒 Files selected for processing (6)
.gitignoredocs/adr-001-multi-packet-streaming-response-api.mddocs/contents.mddocs/execplans/issue-287-inventory-trait-bounds-expecting-frame-vec-u8.mddocs/frame-vec-u8-inventory.mddocs/v0-2-0-to-v0-3-0-migration-guide.md
…nd migration guide - Improved wording for clarity and expanded abbreviations in frame-vec-u8-inventory.md - Fixed formatting issues and link relative path for docs - Corrected typos and code reference in migration guide - These changes improve documentation accuracy and developer understanding without code behavior impact. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
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
Changes
Documentation
Rationale
Validation and checks
Testing plan
Notes for reviewers
◳ Generated by DevBoxer ◰
ℹ️ Tag @devboxerhub to ask questions and address PR feedback
📎 Task: https://www.devboxer.com/task/7afce1d8-b015-472e-b74a-57696a4c91ee
Summary by Sourcery
Add and publish documentation inventorying all remaining
Frame = Vec<u8>trait bounds and APIs, restore the Issue 287 ExecPlan describing that inventory and migration context, and hook these docs into the existing documentation index while making minor readability tweaks to related guides.Documentation:
Frame = Vec<u8>-related trait bounds and payload APIs, including taxonomy, risks, and migration considerations.