Refactor app module into focused submodules

[leynos]

Summary

• Refactors the app module into focused submodules under src/app/ to improve maintainability and keep individual module sizes small. Public API exports remain unchanged.

Changes

• Introduced new submodules:
  • src/app/builder_lifecycle.rs: connection lifecycle builders (on_connection_setup, on_connection_teardown).
  • src/app/builder_protocol.rs: protocol configuration builders (with_protocol, with_message_assembler, message_assembler, protocol, protocol_hooks).
• Updated wiring and module organization:
  • src/app/mod.rs now declares the new submodules and reuses existing builder logic via these focused modules.
  • Core builder functionality is preserved, but responsibilities are split across the new modules for clarity.
• Tests and fixtures:
  • Moved test frame codec fixtures to src/test_helpers/frame_codec.rs and re-exported under src/test_helpers.rs to support frame-handling tests.
  • Updated wiring in unit tests to reference the new fixtures module.
• Docs/Changelog:
  • CHANGELOG.md updated to describe the refactor and the move of ~13 focused files under src/app/ while keeping public API unchanged.

Details

• Builder lifecycle (src/app/builder_lifecycle.rs):
  • on_connection_setup: register a callback invoked when a new connection is established.
  • on_connection_teardown: register a callback invoked when a connection is closed.
• Builder protocol (src/app/builder_protocol.rs):
  • with_protocol: install a WireframeProtocol implementation.
  • with_message_assembler: install a MessageAssembler implementation.
  • message_assembler: query installed MessageAssembler.
  • protocol: return installed protocol, if any.
  • protocol_hooks: derive hooks from installed protocol or default no-op hooks.
• Tests/fixtures:
  • Introduced a shared test frame codec fixture under src/test_helpers/frame_codec.rs used by frame handling tests.
  • Fixture wiring adjusted to ensure compatibility with the new module layout.

Migration notes

• No public API changes. Existing user code that uses WireframeApp builder APIs should continue to compile without changes.
• If you referenced builder methods previously declared in src/app/builder.rs, they are now organized under the new submodules (builder_lifecycle and builder_protocol).

Test plan

• Run cargo test to ensure all unit tests pass.
• Specifically run frame-handling tests that rely on the new test fixtures to verify compatibility with the new module layout.

◳ Generated by DevBoxer ◰

---

ℹ️ Tag @devboxerhub to ask questions and address PR feedback

📎 Task: https://www.devboxer.com/task/74c9a10a-b985-4a32-a5fd-2b0cd06e294c

📝 Closes #291

[sourcery-ai]

Reviewer's Guide

Refactors the WireframeApp builder into lifecycle and protocol-focused submodules, introduces shared frame codec test fixtures reusable across unit and integration tests, slightly tightens a fragmenter assertion, and updates docs/metadata to reflect the new layout while preserving the public API.

Class diagram for shared test frame codec fixture

classDiagram
    class FrameCodec {
      <<trait>>
      +wrap_payload(payload)
      +decoder()
      +encoder()
      +frame_payload(frame)
      +correlation_id(frame)
      +max_frame_length()
    }

    class TestFrame {
      +u8 tag
      +Vec_u8_ payload
    }

    class TestAdapter {
      +usize max_frame_length
      +decode(src)
      +encode(item, dst)
    }

    class TestCodec {
      +usize max_frame_length
      +AtomicUsize counter
      +new(max_frame_length)
      +wraps()
      +decoder()
      +encoder()
      +wrap_payload(payload)
      +frame_payload(frame)
      +correlation_id(frame)
      +max_frame_length()
    }

    class CombinedCodec {
      +new(decoder, encoder)
    }

    class ResponseContext {
      +serializer
      +framed
      +fragmentation
      +codec
    }

    FrameCodec <|.. TestCodec
    TestAdapter ..> TestFrame : encodes_decodes
    TestCodec o--> TestAdapter : uses
    TestCodec --> TestFrame : produces
    CombinedCodec o--> TestAdapter : wraps
    ResponseContext --> FrameCodec : generic_over
    ResponseContext --> CombinedCodec : uses

Loading

File-Level Changes

Change	Details	Files
Split WireframeApp builder responsibilities into lifecycle and protocol-focused submodules while keeping the external builder API stable.	Removed connection lifecycle methods (on_connection_setup, on_connection_teardown) from the main builder implementation and moved them into a new lifecycle-focused module. Removed protocol-related methods (with_protocol, with_message_assembler, message_assembler, protocol, protocol_hooks) from the main builder implementation and moved them into a new protocol-focused module. Updated the app module to declare and re-export the new submodules so callers still access these builder APIs via the existing app surface.	src/app/builder.rs src/app/builder_lifecycle.rs src/app/builder_protocol.rs src/app/mod.rs
Extracted a reusable frame codec fixture for tests and rewired frame-handling tests to depend on it.	Removed the inlined test frame codec types from the frame handling test module and replaced usages with the shared test_frame_codec fixture. Added a new tests::fixtures module in the main crate that paths through to the integration-test fixture file so unit tests can reuse it. Ensured the new fixture compiles and is constructible via a small local test in the fixture module.	src/app/frame_handling.rs tests/fixtures/mod.rs tests/fixtures/test_frame_codec.rs src/tests/mod.rs src/tests/fixtures/mod.rs src/lib.rs
Tightened and clarified tests and documentation around fragmentation and the app module layout.	Strengthened a fragmenter test to assert that the payload is empty using is_empty() rather than slice equality. Adjusted an execution plan document’s formatting to improve readability of mitigation notes. Updated the changelog and roadmap docs to describe the app module refactor and to reference the new src/app/ layout.	src/fragment/tests.rs docs/execplans/migrate-from-cucumber-to-rstest-bdd.md CHANGELOG.md docs/roadmap.md

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#291	Refactor the oversized app module so that its responsibilities are split into focused submodules (e.g., packet/envelope/builder/protocol/io/connection or equivalent), bringing individual file sizes (including the former src/app.rs) under the 400-line limit.	✅
#291	Update module declarations, wiring, and documentation (including src/app/mod.rs and related docs) to reference the new submodules and reflect the refactored app module structure.	✅

Possibly linked issues

• #N/A: PR implements the modularisation pattern for app/builder and frame handling tests, directly addressing part of the refactor request.

---

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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[coderabbitai]

Summary by CodeRabbit

Release Notes

• New Features

  • Added connection lifecycle builder methods for setup and teardown callbacks
  • Added protocol and message assembler configuration options to the builder API
  • Expanded test utilities with new frame codec helpers

• Refactor

  • Reorganised application module into focused components to improve code maintainability
  • Updated documentation to reflect new directory structure

Walkthrough

The PR decomposes src/app.rs by extracting builder methods into focused submodules: builder_lifecycle.rs for connection lifecycle management and builder_protocol.rs for protocol configuration. Test frame codec utilities are consolidated into src/test_helpers/frame_codec.rs with public re-exports. The public API surface remains accessible through module organisation rather than removal.

Changes

Cohort / File(s)	Summary
Module Refactoring src/app/builder.rs, src/app/builder_lifecycle.rs, src/app/builder_protocol.rs, src/app/mod.rs	Decomposed builder.rs by extracting lifecycle methods (on_connection_setup, on_connection_teardown) into builder_lifecycle.rs and protocol methods (with_protocol, with_message_assembler, protocol_hooks) into builder_protocol.rs. Updated module declarations to maintain public API accessibility.
Test Fixture Consolidation src/test_helpers.rs, src/test_helpers/frame_codec.rs	Created new frame_codec module containing TestFrame, TestCodec, and TestAdapter implementations with Encoder/Decoder/FrameCodec trait implementations. Re-exported these types from test_helpers for cross-module availability.
Test Updates src/app/frame_handling.rs, src/fragment/tests.rs	Updated frame_handling tests to use CombinedCodec and shared TestCodec fixtures instead of bespoke implementations. Simplified assertion in fragmenter test to use is_empty().
Documentation CHANGELOG.md, docs/roadmap.md, docs/execplans/migrate-from-cucumber-to-rstest-bdd.md	Recorded module refactoring in CHANGELOG. Updated handler reference from src/app.rs to src/app/ directory. Reformatted Risk 3 mitigation text across lines.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Refactor app.rs: decompose module exceeding 400-line limit #291: This PR directly addresses the refactoring requirement to decompose src/app.rs and split it into submodules (builder_lifecycle.rs, builder_protocol.rs, etc.) to enforce the 400-line file size limit.
• Reduce file size hotspots by splitting into submodules #428: The changes align with refactoring and splitting the src/app module into focused submodules to reduce file-size hotspots.

Possibly related PRs

• Document codec testing roadmap #414: The consolidation of test frame codec utilities (TestCodec, TestAdapter, TestFrame) and their re-export from test_helpers relates to the codec-aware testing infrastructure and observability for such fixtures.
• Add MessageAssembler hook trait, headers, tests, and app integration #415: The extraction of protocol and message assembler builder methods into builder_protocol.rs directly corresponds to the MessageAssembler trait introduction and WireframeApp builder methods initially developed in that PR.

Poem

🏗️ Large modules fall, split with care,
Builder methods find their lair,
Test fixtures shared, no duplication,
API preserved—just reorganisation!
Four hundred lines become the rule,
Refactoring keeps the codebase cool.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarises the primary change: refactoring the app module into focused submodules, directly reflecting the main objective of the PR.
Description check	✅ Passed	The description thoroughly explains the refactoring objectives, new submodules introduced (builder_lifecycle.rs and builder_protocol.rs), test fixture reorganisation, and migration notes confirming public API compatibility.
Linked Issues check	✅ Passed	The PR successfully addresses issue #291 by decomposing src/app.rs into focused submodules under src/app/, splitting builder functionality across builder_lifecycle.rs and builder_protocol.rs whilst maintaining the 400-line file guideline and preserving public API exports.
Out of Scope Changes check	✅ Passed	All changes remain within scope: the refactoring splits app module responsibilities, moves test fixtures, updates documentation, and maintains existing APIs. No extraneous changes or unrelated features are introduced.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor-app-module-splitting-6km9ns

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sourcery-ai]

Hey - I've left some high level feedback:

• The new src/tests module is only gated behind #[cfg(test)] while test_helpers uses #[cfg(any(test, feature = "test-helpers"))]; if these fixtures are meant to be reused like the other helpers, consider aligning the cfg so you don’t end up with two parallel, slightly different test-helper mechanisms.
• In src/tests/fixtures/mod.rs you use a relative #[path = "../../../tests/fixtures/test_frame_codec.rs"], which is quite brittle to directory layout changes; consider moving the implementation into src/ and importing it from tests/ (rather than the other way around) to avoid deep relative paths.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new `src/tests` module is only gated behind `#[cfg(test)]` while `test_helpers` uses `#[cfg(any(test, feature = "test-helpers"))]`; if these fixtures are meant to be reused like the other helpers, consider aligning the cfg so you don’t end up with two parallel, slightly different test-helper mechanisms.
- In `src/tests/fixtures/mod.rs` you use a relative `#[path = "../../../tests/fixtures/test_frame_codec.rs"]`, which is quite brittle to directory layout changes; consider moving the implementation into `src/` and importing it from `tests/` (rather than the other way around) to avoid deep relative paths.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[leynos]

@coderabbitai Have the following now been resolved?

• The new src/tests module is only gated behind #[cfg(test)] while test_helpers uses #[cfg(any(test, feature = "test-helpers"))]; if these fixtures are meant to be reused like the other helpers, consider aligning the cfg so you don’t end up with two parallel, slightly different test-helper mechanisms.
• In src/tests/fixtures/mod.rs you use a relative #[path = "../../../tests/fixtures/test_frame_codec.rs"], which is quite brittle to directory layout changes; consider moving the implementation into src/ and importing it from tests/ (rather than the other way around) to avoid deep relative paths.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Check the cfg attribute on src/tests module in src/lib.rs
echo "=== src/lib.rs (tests module declaration) ==="
rg -A 1 "mod tests" src/lib.rs

# Check src/tests/fixtures/mod.rs for the path attribute
echo -e "\n=== src/tests/fixtures/mod.rs ==="
cat src/tests/fixtures/mod.rs

# Search for test_helpers module to compare cfg attributes
echo -e "\n=== test_helpers module declarations ==="
rg -n "mod test_helpers" src/

# Also check if there's a test_helpers file or directory
echo -e "\n=== test_helpers files ==="
fd test_helpers

Length of output: 371

---

🏁 Script executed:

#!/bin/bash

# Check the entire src/lib.rs to see the tests module
echo "=== src/lib.rs content (last 50 lines) ==="
tail -n 50 src/lib.rs

# Check if src/tests directory exists
echo -e "\n=== src/tests directory structure ==="
if [ -d "src/tests" ]; then
    tree src/tests
else
    echo "src/tests directory does not exist"
fi

# Check the test_helpers module cfg in src/lib.rs
echo -e "\n=== test_helpers module declaration in src/lib.rs ==="
rg -B 2 -A 1 "pub mod test_helpers" src/lib.rs

# Check if there's a tests module in src/
echo -e "\n=== Files in src/ ==="
ls -la src/ | grep -E "test|mod"

Length of output: 1671

---

🏁 Script executed:

#!/bin/bash

# Let's search for all files that were mentioned in the AI summary as being changed
echo "=== Searching for src/tests/mod.rs ==="
find . -path "*/src/tests/mod.rs" 2>/dev/null

echo -e "\n=== Searching for src/tests/fixtures/mod.rs ==="
find . -path "*/src/tests/fixtures/mod.rs" 2>/dev/null

# Check the git diff to see what was actually changed in the PR
echo -e "\n=== Git diff for src/lib.rs ==="
git diff origin/main...HEAD -- src/lib.rs | head -n 30

# Check for any test module additions
echo -e "\n=== All src/tests related files in git diff ==="
git diff origin/main...HEAD --name-only | grep -E "src/tests|tests/fixtures"

# Also check what was actually added in src/lib.rs
echo -e "\n=== Complete src/lib.rs (looking for tests module) ==="
cat src/lib.rs | grep -A 2 -B 2 "tests"

Length of output: 225

---

🏁 Script executed:

#!/bin/bash

# Let's check the tests/fixtures directory
echo "=== tests/fixtures directory structure ==="
if [ -d "tests/fixtures" ]; then
    ls -la tests/fixtures/
else
    echo "tests/fixtures directory does not exist"
fi

# Check if the test_frame_codec.rs exists
echo -e "\n=== tests/fixtures/test_frame_codec.rs existence ==="
if [ -f "tests/fixtures/test_frame_codec.rs" ]; then
    echo "File exists"
    head -n 20 tests/fixtures/test_frame_codec.rs
else
    echo "File does not exist"
fi

# Let's look at what files were changed in the PR
echo -e "\n=== Files changed (last commit) ==="
git diff HEAD~1 --name-only

# Check all files in the repository matching the pattern
echo -e "\n=== All files matching src/tests or tests/fixtures ==="
find . -type f \( -path "*/src/tests/*" -o -path "*/tests/fixtures/*" \) 2>/dev/null

# Check src/lib.rs more carefully
echo -e "\n=== Full src/lib.rs ==="
cat src/lib.rs

Length of output: 5001

---

🏁 Script executed:

#!/bin/bash

# Check if src/tests/mod.rs and src/tests/fixtures/mod.rs exist
echo "=== Checking src/tests/mod.rs ==="
if [ -f "src/tests/mod.rs" ]; then
    cat src/tests/mod.rs
else
    echo "File does not exist"
fi

echo -e "\n=== Checking src/tests/fixtures/mod.rs ==="
if [ -f "src/tests/fixtures/mod.rs" ]; then
    cat src/tests/fixtures/mod.rs
else
    echo "File does not exist"
fi

# Check what the actual changes were in the last commit
echo -e "\n=== Git diff for src/lib.rs ==="
git show HEAD:src/lib.rs | tail -n 10

# Check src/test_helpers/frame_codec.rs
echo -e "\n=== Checking src/test_helpers/frame_codec.rs ==="
if [ -f "src/test_helpers/frame_codec.rs" ]; then
    head -n 40 src/test_helpers/frame_codec.rs
else
    echo "File does not exist"
fi

# Let's see the diff for the files mentioned
echo -e "\n=== Git diff for src/tests/mod.rs ==="
git diff HEAD~1 HEAD -- src/tests/mod.rs

echo -e "\n=== Git diff for src/tests/fixtures/mod.rs ==="
git diff HEAD~1 HEAD -- src/tests/fixtures/mod.rs

Length of output: 2175

---

🏁 Script executed:

#!/bin/bash

# Let's verify that the test frame codec is now in test_helpers
echo "=== Checking src/test_helpers.rs ==="
cat src/test_helpers.rs

# Verify that src/app/frame_handling.rs is using the new location
echo -e "\n=== Checking src/app/frame_handling.rs imports ==="
rg -n "test_frame_codec|TestCodec|frame_codec" src/app/frame_handling.rs | head -n 20

# Verify tests/fixtures/mod.rs doesn't export test_frame_codec anymore
echo -e "\n=== Checking tests/fixtures/mod.rs ==="
cat tests/fixtures/mod.rs

Length of output: 4429

---

@leynos: Yes, both issues have been resolved!

The src/tests module and src/tests/fixtures/mod.rs have been deleted, which eliminates both concerns:

1. cfg attribute consistency: The separate src/tests module with #[cfg(test)] no longer exists. The test frame codec now lives in src/test_helpers/frame_codec.rs and is gated behind #[cfg(any(test, feature = "test-helpers"))], maintaining consistency with the existing test helpers.

2. Brittle relative path: The problematic #[path = "../../../tests/fixtures/test_frame_codec.rs"] has been removed. The implementation is now properly located in src/test_helpers/frame_codec.rs and imported through normal module resolution.

The refactor successfully consolidated all test utilities under the existing test_helpers module, avoiding the creation of parallel test-helper mechanisms.