refactor: split main.rs into modules #44
Conversation
Split the 1256-line main.rs into a modular structure: - cli.rs: CLI definitions (Cli, Commands, Algorithm, OutputFormat) - macros.rs: 6 dispatch macros (kem_keygen!, dsa_keygen!, etc.) - io.rs: encode/decode helpers and write_secret_file - commands/: 7 command handlers (keygen, encaps, decaps, sign, verify, info, completions) - main.rs: entry point with mod declarations and match dispatch only No functional changes. All 30 tests pass.
Summary of ChangesHello @kiyoaki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
External Tool Benchmark (arm64-macos)Kylix Benchmark ComparisonML-KEM-768
|
There was a problem hiding this comment.
Pull request overview
Refactors kylix-cli by splitting the previously large main.rs into focused modules while keeping runtime behavior the same.
Changes:
- Extracted CLI parsing/types into
cli.rs, I/O helpers intoio.rs, and dispatch macros intomacros.rs - Moved each
cmd_*handler intocommands/and re-exported viacommands/mod.rs - Updated
bench.rsimports and documented completion of the refactor task inPLANS.md
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| kylix-cli/src/main.rs | Reduced to entrypoint wiring: module declarations + command dispatch |
| kylix-cli/src/cli.rs | Centralized clap CLI types and algorithm metadata/detection helpers |
| kylix-cli/src/macros.rs | Moved algorithm dispatch macros out of main.rs |
| kylix-cli/src/io.rs | Moved encoding/decoding and secret-file writing helpers out of main.rs |
| kylix-cli/src/commands/mod.rs | Command-module registry + re-exports for dispatching |
| kylix-cli/src/commands/keygen.rs | Extracted key generation command handler |
| kylix-cli/src/commands/encaps.rs | Extracted encapsulation command handler |
| kylix-cli/src/commands/decaps.rs | Extracted decapsulation command handler |
| kylix-cli/src/commands/sign.rs | Extracted signing command handler |
| kylix-cli/src/commands/verify.rs | Extracted signature verification command handler |
| kylix-cli/src/commands/info.rs | Extracted algorithm info display command |
| kylix-cli/src/commands/completions.rs | Extracted shell completions generator |
| kylix-cli/src/bench.rs | Updated Algorithm import path after refactor |
| PLANS.md | Marked the module-splitting task as completed/moved |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
External Tool Benchmark (x86_64-linux)Kylix Benchmark ComparisonML-KEM-768
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the project structure by refactoring the CLI tool and splitting the monolithic main.rs into logical modules, making it easier to navigate and maintain. However, a critical security vulnerability was identified: sensitive cryptographic material (secret keys and shared secrets) could leak into heap memory due to non-zeroized intermediate buffers. Specifically, the io.rs module and command handlers for decapsulation, signing, and key generation require updates to ensure all copies of secrets are properly zeroized. Additionally, there are minor code cleanup opportunities related to unused imports that, if addressed, will further enhance code quality.
- Fully-qualify rng(), Zeroizing, anyhow!() in dispatch macros to eliminate hidden caller-side import dependencies - Add sync_all() before rename in write_secret_file for crash safety - Clean up temp file on rename failure to avoid leaving secret material - Remove now-unused anyhow/rng imports from command modules and io.rs
|
/gemini review |
External Tool Benchmark (arm64-macos)Kylix Benchmark ComparisonML-KEM-768
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
External Tool Benchmark (x86_64-linux)Kylix Benchmark ComparisonML-KEM-768
|
There was a problem hiding this comment.
Code Review
This is an excellent refactoring that dramatically improves the structure and maintainability of the CLI crate. Splitting the monolithic main.rs into a clean, modular structure with cli.rs, io.rs, macros.rs, and the commands/ directory is a huge step forward. The new organization is logical and follows standard Rust practices for CLI applications. Great job! There is one critical issue regarding a missing use statement in the new io.rs file that needs to be addressed.
Update comments referencing nonexistent `detect_kem_algorithm` to the actual method names: `detect_kem_from_pub_key` and `detect_kem_from_sec_key`.
External Tool Benchmark (arm64-macos)Kylix Benchmark ComparisonML-KEM-768
|
External Tool Benchmark (x86_64-linux)Kylix Benchmark ComparisonML-KEM-768
|
1 participant
Summary
main.rsinto modular structure:cli.rs,macros.rs,io.rs, andcommands/directory with 7 command handlersmain.rsis now ~100 lines (entry point with mod declarations and match dispatch only)Module Structure
Test plan
cargo fmt --all— cleancargo clippy --workspace --all-features -- -D warnings— cleancargo clippy --workspace --no-default-features -- -D warnings— cleancargo test --workspace --all-features— 30/30 passCloses PLANS.md task [M-2].