Skip to content

fix: not include all dependencies in the generated code of the server#14

Merged
Bechma merged 1 commit intocyberfabric:mainfrom
Bechma:bugfix-deps
Mar 18, 2026
Merged

fix: not include all dependencies in the generated code of the server#14
Bechma merged 1 commit intocyberfabric:mainfrom
Bechma:bugfix-deps

Conversation

@Bechma
Copy link
Collaborator

@Bechma Bechma commented Mar 18, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Corrected dependency handling in server structure generation to use original dependencies instead of merged dependencies when preparing the main template.

Signed-off-by: Bechma <19294519+Bechma@users.noreply.github.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 18, 2026

📝 Walkthrough

Walkthrough

A single parameter in generate_server_structure was modified to pass pre-merged dependencies (current_dependencies) to prepare_cargo_server_main instead of the final merged dependencies from cargo_toml. The control flow and final Cargo.toml construction remain unchanged; only the intermediate data passed to the main.rs template differs.

Changes

Cohort / File(s) Summary
Dependency Parameter Handling
crates/cli/src/common.rs
Modified the argument passed to prepare_cargo_server_main from &cargo_toml.dependencies (post-merge) to current_dependencies (pre-merge), changing which dependency state is used when preparing the server's main.rs template.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A little tweak to which deps we send,
Pre-merge instead of merged, my friend,
The flow stays smooth, the logic clean,
Dependencies dance in between!
Hopping toward perfection, one change at a time! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main change: modifying which dependencies are included in generated server code, changing from merged dependencies to pre-merge dependencies.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/cli/src/common.rs (1)

241-268: Add a regression test for this dependency-selection behavior.

This is easy to accidentally revert. Please add a test asserting generated src/main.rs includes imports from current_dependencies and excludes deps introduced only by create_required_deps.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/cli/src/common.rs` around lines 241 - 268, Add a regression test to
ensure generate_server_structure selects dependencies correctly: call
generate_server_structure (or its helpers) with a controlled
current_dependencies and a create_required_deps that would add extra deps, then
read the produced "src/main.rs" (created via create_file_structure) and assert
that imports rendered by prepare_cargo_server_main include symbols from
current_dependencies but do NOT include symbols from deps only returned by
create_required_deps; target the behavior in generate_server_structure,
prepare_cargo_server_main, create_required_deps and create_file_structure so the
test fails if dependency-selection is accidentally reverted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@crates/cli/src/common.rs`:
- Around line 241-268: Add a regression test to ensure generate_server_structure
selects dependencies correctly: call generate_server_structure (or its helpers)
with a controlled current_dependencies and a create_required_deps that would add
extra deps, then read the produced "src/main.rs" (created via
create_file_structure) and assert that imports rendered by
prepare_cargo_server_main include symbols from current_dependencies but do NOT
include symbols from deps only returned by create_required_deps; target the
behavior in generate_server_structure, prepare_cargo_server_main,
create_required_deps and create_file_structure so the test fails if
dependency-selection is accidentally reverted.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ef1193ba-32b2-4a11-a7dc-663bfca0d367

📥 Commits

Reviewing files that changed from the base of the PR and between 0d288f5 and d982059.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • crates/cli/src/common.rs

@Bechma Bechma merged commit f84e793 into cyberfabric:main Mar 18, 2026
2 checks passed
@Bechma Bechma deleted the bugfix-deps branch March 23, 2026 15:26
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