Skip to content

Docs(execplans): correct stale codec path in issue 287 plan - #521

Draft
leynos wants to merge 1 commit into
mainfrom
inventory-trait-bounds-frame-vec-u8-lm8fpf
Draft

leynos wants to merge 1 commit into
mainfrom
inventory-trait-bounds-frame-vec-u8-lm8fpf

Conversation

@leynos

@leynos leynos commented Apr 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

Corrects two stale src/codec.rs path references in the issue-287 ExecPlan to
src/codec/mod.rs.

This branch originally proposed a Status: PROPOSED draft of that ExecPlan.
That draft has been superseded: PR #522 (merged 2026-04-12) published the
issue-287 deliverable docs/frame-vec-u8-inventory.md, recovered the same
ExecPlan file from commit c0cbc75, and advanced it to Status: COMPLETE,
closing issue #287. Rebasing this branch onto current origin/main therefore
left the branch's commit empty, and Git dropped it.

What survives is a genuine, unrelated defect in the merged document.

Change description

815dad65 ("Normalize Rust module layout (#710)", 2026-10-11) moved
src/codec.rs to src/codec/mod.rs. The ExecPlan, merged months earlier, still
cited the old path in two places:

  • "Context and orientation" — the bullet naming where FrameCodec is
    defined.
  • "Surprises & Discoveries" — the observation that the codec and protocol
    trait layers are generic over Self::Frame, citing src/codec.rs alongside
    src/hooks.rs.

Both claims remain true; only the path drifted. The plan's own constraints
require it to "reflect the current repository layout rather than historical
issue context", and its published companion docs/frame-vec-u8-inventory.md
already cites the current path — so the two artefacts contradicted each other.

The correction is recorded in the plan's revision note so the provenance of the
edit is not lost.

Evidence

Verified against origin/main (a974dc76):

  • src/codec/mod.rs:64 — pub trait FrameCodec: Send + Sync + Clone + 'static
    with type Frame: Send + Sync + 'static.
  • src/codec.rs does not exist; 815dad65 is the commit that renamed it.
  • Adjacent evidence cited on the same lines is unchanged and still accurate:
    src/hooks.rs:28 (pub trait WireframeProtocol), src/hooks.rs:30
    (type Frame: FrameLike), and src/connection/mod.rs:115
    (FrameLike + CorrelatableFrame + Packet).

Scope

Documentation only. One file, no runtime code, no dependency or manifest
changes:

 docs/execplans/issue-287-inventory-trait-bounds-expecting-frame-vec-u8.md
   | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

Validation

CI on this PR is fully green against head 43a19f7b:

Check Result
build-test (cargo test workspace) pass, 16m24s
Analyze (rust) pass, 6m24s
Analyze (actions) / Analyze (python) pass
CodeQL pass
CodeScene Code Health Review pass
Gecko Security Review pass

That green build-test is the authoritative Rust gate for this exact commit,
so the local-host gaps below do not leave the change ungated.

Local runs against the same candidate 43a19f7b, on a host sharing a
compile-admission pool with several other agents:

  • make check-fmt — pass (cargo fmt --check clean; mdtablefix reports
    122 files unchanged)
  • make markdownlint — pass, including the typos-config-builder spelling
    gate; 122 files, 0 issues
  • make test, make typecheck, make lint — no verdict

Those three are reported honestly rather than claimed as passes. This host
shares a compile-admission pool with several other agents, and gates were
repeatedly cut off by runner ceilings while queued behind ~200–400 pending
compilations from unrelated repositories:

  • make typecheck — completed on an earlier run at 3,408 s with exit 0, but
    that run predates the final commit; not re-claimed here.
  • make lint — stopped by the runner ceiling while still compiling
    dependencies; the log contains zero error lines and never reached Clippy
    over the workspace crates.
  • make test — one run was aborted by the admission supervisor refusing to
    launch a compiler for the third-party regex-automata crate
    (build-limits: the supervisor refused: launch-failed); a second run reached
    the test phase and completed 13 test binaries with 0 failures, 0 errors,
    0 panics
    , but was stopped by the runner ceiling while the trybuild UI
    fixture suite was still compiling its nested workspaces.

Why the missing verdicts do not mask a defect here. This commit changes one
Markdown file and nothing else. The tree is otherwise byte-identical to
origin/main (a974dc76), so no Rust code, manifest, build script, or test
fixture differs from a commit that is already merged and green. Nothing in the
build reads docs/: no include_str!, include_bytes!, rust-embed stanza,
or build.rs references that tree, and no test loads the ExecPlans. The two
gates that completed in full — formatting and Markdown linting, the latter with
the repository's typos-config-builder spelling gate over 122 files — are the
ones that actually cover a Markdown-only diff. make test cannot distinguish
this commit from its parent, so its verdict carries no information about this
change either way.

The completed local gates are re-run by CI on this PR in any case, and that
run is green; it is the authoritative result.

Impact

None at runtime. Restores internal consistency between the ExecPlan and the
inventory document it describes, and removes a dead path that would misdirect a
future reader tracing FrameCodec.

Notes for reviewers

  • This PR no longer closes issue Analysis: Inventory trait bounds expecting Frame = Vec<u8> #287. That issue was closed by Publish Frame Vec<u8> inventory docs and recover ExecPlan #522, which
    landed the actual inventory work. The Closes trailer from the original
    description has been removed.
  • This PR must not close epic Epic: Zero-copy frame API (breaking) – adopt Bytes as Frame #284. The epic tracks the Bytes migration
    itself, which is untouched here and remains open.
  • The rebase onto current origin/main produced no conflicts; the branch's
    original commit became empty because its content was already on main by a
    different route.
  • Why only this ExecPlan? Roughly a dozen other ExecPlans under docs/
    (9-1-1, 9-1-2, 9-1-3, 9-3-1, and others) also still cite src/codec.rs.
    Those are deliberate historical records of plans written against the layout
    at the time, and rewriting them would falsify what was planned. This one is
    different: it is marked COMPLETE, its "Context and orientation" section is
    a present-tense description of the current repository, and its own
    constraints require it to track the current layout. It is also the plan whose
    companion inventory document cites the current path, so leaving it alone
    would keep the pair inconsistent. A blanket sweep of all ExecPlans is
    deliberately out of scope for this PR.

References

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Apr 10, 2026 •

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch inventory-trait-bounds-frame-vec-u8-lm8fpf
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds a new execplan document that inventories and plans around Frame = Vec<u8> trait bounds and APIs, defining taxonomy, risks, validation steps, and a staged plan to support a future Bytes migration without changing runtime code.

Sequence diagram for current Vec_u8 payload flow through codec, envelope, middleware, and hooks

sequenceDiagram
    participant Codec
    participant PacketParts
    participant Envelope
    participant FrameHandlingResponse
    participant ServiceRequest
    participant ServiceResponse
    participant ClientBeforeSendHook
    participant DLQSender

    Codec->>PacketParts: decode() returns PacketParts{ payload: Vec_u8 }
    PacketParts->>Envelope: new() with payload: Vec_u8
    Envelope-->>FrameHandlingResponse: pass Envelope with payload: Vec_u8
    FrameHandlingResponse->>ServiceRequest: ServiceRequest::new(env.payload, env.correlation_id)
    ServiceRequest-->>ServiceRequest: frame_mut() returns &mut Vec_u8
    ServiceRequest-->>ServiceResponse: processed payload Vec_u8
    ServiceResponse-->>ClientBeforeSendHook: invoke BeforeSendHook(&mut Vec_u8)
    ClientBeforeSendHook-->>ServiceResponse: possibly mutated Vec_u8
    ServiceResponse-->>DLQSender: push_dlq.send(Vec_u8) on error path
Loading

File-Level Changes

Change Details Files
Add an execplan markdown document describing how to inventory and classify all Vec<u8>-coupled frame and payload surfaces and how that feeds into a future Bytes migration.
  • Introduce a purpose and scope section for issue 287 and its relationship to epic 284, clarifying that the work is an inventory and planning effort rather than the migration itself.
  • Define constraints, tolerances, risks, validation and acceptance criteria, including concrete CI commands and optional rstest usage for any exploratory tests added later.
  • Document current evidence of Vec<u8> coupling across middleware, envelopes, frame handling, builders, hooks, correlation traits, and existing design docs, with guidance on classification as frame-bound, payload-bound, internal-only, or docs/tests-only.
  • Lay out a staged plan of work (Stages A–E) with concrete steps for scanning the codebase, classifying findings, tracing runtime flows, optionally adding tests, and updating epics/roadmap entries.
  • Add supporting sections for exploratory testing strategy, interfaces and dependencies, idempotence and recovery, progress tracking, surprises & discoveries, decision log, and outcomes/retrospective to keep the execplan as a living document.
docs/execplans/issue-287-inventory-trait-bounds-expecting-frame-vec-u8.md

Assessment against linked issues

Issue Objective Addressed Explanation
#284 Produce a design proposal for a Bytes-based Frame API (e.g., Bytes-only vs dual support), including options, risks, and trade-offs for migrating from Vec. ❌ The PR adds an execplan document that inventories where Frame/Vec assumptions exist and how to catalog them. It does not propose a concrete Bytes-based Frame design or compare design options, nor does it analyze trade-offs of Bytes vs alternatives.
#284 Document a migration and compatibility strategy for downstream users, including a release/rollout plan for the breaking change and a migration guide outline for a major version bump. ❌ The PR focuses on planning an internal inventory of Vec-bound surfaces and scoping future work. It does not describe how downstream users should migrate, does not define compatibility strategies, and does not specify a release or rollout plan for the breaking change.
#284 Define a benchmarking plan and baseline measurements to validate zero-copy decoding and performance goals for the Bytes-based Frame API. ❌ The PR does not mention benchmarks, performance goals, or measurement methodology. It only introduces a documentation plan for cataloguing current Vec couplings and does not specify any benchmarking plan or baseline metrics.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Commit 815dad6 ("Normalize Rust module layout (#710)") moved
`src/codec.rs` to `src/codec/mod.rs`, but the issue-287 ExecPlan still
cited the old path in two places: the "Context and orientation" summary
of where `FrameCodec` is defined, and the "Surprises & Discoveries"
observation that the codec layer is generic over `Self::Frame`.

Both claims remain true; only the path had drifted. This contradicted
the plan's own constraint to "reflect the current repository layout
rather than historical issue context", and its published companion
`docs/frame-vec-u8-inventory.md` already cited the current path, so the
two artefacts disagreed with each other.

Verified `FrameCodec` is declared at `src/codec/mod.rs:64`, that no
`src/codec.rs` exists, and that the adjacent `src/hooks.rs` and
`src/connection/mod.rs` evidence cited alongside it is unchanged.
Recorded the correction in the plan's revision note.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@leynos
leynos force-pushed the inventory-trait-bounds-frame-vec-u8-lm8fpf branch from c0cbc75 to 43a19f7 Compare October 23, 2026 22:08
@leynos leynos changed the title Inventory: Frame Vec<u8> bounds, APIs, and plan (issue 287) Docs(execplans): correct stale codec path in issue 287 plan Oct 23, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant