Skip to content

feat(groq): CLI tool that solves a tiny knapsack problem to help post‑apocalypse scavengers pick the most valuable items within a weight limit.#3690

Open
polsala wants to merge 1 commit intomainfrom
ai/groq-20260309-1052
Open

feat(groq): CLI tool that solves a tiny knapsack problem to help post‑apocalypse scavengers pick the most valuable items within a weight limit.#3690
polsala wants to merge 1 commit intomainfrom
ai/groq-20260309-1052

Conversation

@polsala
Copy link
Owner

@polsala polsala commented Mar 9, 2026

Implementation Summary

  • Utility: nightly-scavenger-optimize
  • Provider: groq
  • Location: rust-utils/nightly-nightly-scavenger-optimize
  • Files Created: 5
  • Description: CLI tool that solves a tiny knapsack problem to help post‑apocalypse scavengers pick the most valuable items within a weight limit.

Rationale

  • Automated proposal from the Groq generator delivering a fresh community utility.
  • This utility was generated using the groq AI provider.

Why safe to merge

  • Utility is isolated to rust-utils/nightly-nightly-scavenger-optimize.
  • README + tests ship together (see folder contents).
  • No secrets or credentials touched.
  • All changes are additive and self-contained.

Test Plan

  • Follow the instructions in the generated README at rust-utils/nightly-nightly-scavenger-optimize/README.md
  • Run tests located in rust-utils/nightly-nightly-scavenger-optimize/tests/

Links

  • Generated docs and examples committed alongside this change.

Mock Justification

  • Not applicable; generator did not introduce new mocks.

…‑apocalypse scavengers pick the most valuable items within a weight limit.
@polsala
Copy link
Owner Author

polsala commented Mar 9, 2026

🤖 Review by GROQ Agent

✅ What's solid

  • Clear separation of concerns – the core knapsack logic lives in lib.rs while the CLI lives in main.rs. This makes the algorithm reusable and testable.
  • Straight‑forward APIparse_items and optimal_subset are small, pure functions with no hidden side‑effects, which simplifies reasoning and testing.
  • Comprehensive README – the usage example, installation instructions, and test command are all present, giving a good first‑time‑user experience.
  • Deterministic tests – the three test cases cover parsing, a normal optimisation scenario, and the “nothing fits” edge case, providing decent baseline coverage.
  • No external dependencies – the crate stays lightweight and can be built in any environment that supports Rust 2021.

🧪 Tests

  • Integration‑test import noise – the test file lives under tests/, so it is compiled as an integration test. The line use super::super::*; does not resolve to anything useful and can cause a compilation error.
    // Remove the unnecessary import
    // use super::super::*;
  • Error‑path testingparse_items currently panics on malformed input. Add a test that supplies an invalid string and asserts that the function returns an error (or, if you keep the panic, use #[should_panic]). This will protect against regressions when the parsing logic changes.
  • Boundary‑size test – the brute‑force algorithm enumerates subsets using a u64 bitmask, which caps the supported item count at 64. A test that feeds 65 items and verifies that the function gracefully handles the overflow (e.g., returns an empty vec or an explicit error) would document this limitation.
  • Performance sanity check (optional) – for larger inputs you could add a benchmark (using criterion) to ensure the exponential algorithm remains acceptable for the intended “tiny” problem size.

🔒 Security

  • Panic on malformed inputparse_items uses panic! for format errors and expect for integer parsing. If the CLI ever receives untrusted data (e.g., from a script or a web service), a malicious payload could cause the process to abort, which is a denial‑of‑service vector.
    • Recommendation: Change the signature to pub fn parse_items(s: &str) -> Result<Vec<Item>, ParseError> and propagate errors up to main.rs, where you can emit a user‑friendly message and exit with a non‑zero status.
    #[derive(Debug)]
    pub enum ParseError {
        InvalidFormat(String),
        InvalidNumber(String),
    }
    
    pub fn parse_items(s: &str) -> Result<Vec<Item>, ParseError> {
        // … return Err(ParseError::InvalidFormat(p.to_string())) …
    }
  • Bitmask overflow – the loop for mask in 0u64..(1u64 << n) will overflow when n == 64, panicking at runtime. Guard against this early and return an error if items.len() > 64.
    if items.len() > 64 {
        return Err(KnapsackError::TooManyItems);
    }

🧩 Docs / Developer Experience

  • Derive traits instead of manual implItem only needs Clone (and probably Debug, PartialEq for testing). Replace the hand‑rolled Clone with #[derive(Clone, Debug, PartialEq)]. This reduces boilerplate and eliminates the risk of diverging implementations.
    #[derive(Clone, Debug, PartialEq)]
    pub struct Item {
        pub name: String,
        pub weight: u32,
        pub value: u32,
    }
  • README path typo – the installation snippet points to cd utils/rust-utils/nightly-scavenger-optimize, but the actual path in the repository is rust-utils/nightly-nightly-scavenger-optimize. Align the command to avoid confusion.
  • CLI help output – consider adding a --help flag that prints the usage text and exits with status 0. This can be done with a simple check for args.contains(&"--help".to_string()).
  • Cargo metadata – enrich Cargo.toml with a short description, repository URL, and license field. This improves visibility on crates.io should the utility ever be published.
    description = "Tiny 0/1 knapsack CLI for post‑apocalypse scavengers"
    repository = "https://github.com/polsala/ApocalypsAI"
    license = "MIT"

🧱 Mocks / Fakes

  • No external services or I/O abstractions are used, so mocks are unnecessary at the moment.
  • If you later introduce file‑based input (e.g., reading items from a JSON file), consider abstracting the source behind a trait (ItemSource) and providing a mock implementation for unit tests. This will keep the core algorithm testable without touching the filesystem.

Overall impression: the utility is well‑structured, the core algorithm works for the intended small problem size, and the documentation makes it easy to try out. Addressing the panic‑based error handling, tightening the test imports, and adding a few defensive checks will make the crate more robust and production‑ready.

@polsala
Copy link
Owner Author

polsala commented Mar 9, 2026

🤖 Review by GEMINI Agent

✅ What's solid

  • The core logic for solving the knapsack problem is correctly implemented using a brute-force bitmask approach, which is suitable for the stated "tiny" problem size.
  • The separation of concerns between src/lib.rs (core logic) and src/main.rs (CLI interface) is well-executed, promoting reusability and testability.
  • The README.md provides clear and concise instructions for installation, usage, and testing, making the utility immediately accessible.

🧪 Tests

  • The existing tests cover basic parsing and optimal subset selection, including an edge case where no items fit.
  • Consider adding tests for parse_items that specifically trigger the panic conditions (e.g., malformed item string like "2:5" or "2:five:water"). This would confirm the expected error handling behavior, even if it's a panic.
  • Introduce a test case for optimal_subset with multiple items having the same weight/value to ensure stable selection or correct handling of ties.
  • Add a test for an empty input string to parse_items (e.g., "" or ",") to confirm it gracefully handles such inputs.
  • A test for main.rs argument parsing and error messages would be beneficial, perhaps using a crate like assert_cmd for CLI testing. This would verify the print_usage and eprintln calls for invalid arguments.

🔒 Security

  • The parse_items function uses panic! for invalid input formats or non-integer weights/values. While this is a utility, a panic can lead to an ungraceful exit for the user. Consider replacing panic! with Result<T, E> to allow for more robust error handling and user-friendly error messages in main.rs.
    // Example of returning a Result from parse_items
    pub fn parse_items(s: &str) -> Result<Vec<Item>, String> {
        s.split(',')
            .filter(|p| !p.trim().is_empty())
            .map(|p| {
                let parts: Vec<&str> = p.split(':').collect();
                if parts.len() != 3 {
                    return Err(format!("Invalid item format: {}", p));
                }
                let weight = parts[0]
                    .parse::<u32>()
                    .map_err(|_| format!("Invalid weight component in '{}'", p))?;
                let value = parts[1]
                    .parse::<u32>()
                    .map_err(|_| format!("Invalid value component in '{}'", p))?;
                let name = parts[2].to_string();
                Ok(Item { name, weight, value })
            })
            .collect::<Result<Vec<Item>, String>>() // Collect the results
    }
  • The brute-force optimal_subset algorithm has a time complexity of O(2^n), where n is the number of items. While acceptable for a "tiny" knapsack problem, explicitly define or document the maximum n for which this utility is intended to prevent performance issues or resource exhaustion if used with larger inputs. The current mask uses u64, implicitly limiting n to 64 items.

🧩 Docs/DX

  • The README.md is excellent, providing all necessary information for a user to get started.
  • Improve the error handling in parse_items (as suggested in Security) to return a Result instead of panicking. This would allow main.rs to provide more specific and user-friendly error messages for malformed item inputs, enhancing the overall user experience.
  • Ensure consistency in naming: the folder is rust-utils/nightly-nightly-scavenger-optimize, the utility is nightly-scavenger-optimize in the PR body, and the Cargo.toml name is scavenger_optimize. Standardize on one name for the utility across all contexts (folder, binary, documentation). For example, if scavenger_optimize is the intended binary name, the folder could be rust-utils/scavenger-optimize.
  • Consider adding a --help flag using a crate like clap to automatically generate usage information and handle argument parsing more robustly than manual env::args().collect(). This would improve the CLI's discoverability and user experience.

🧱 Mocks/Fakes

  • As a self-contained utility with no external dependencies or complex interactions, the absence of mocks or fakes is appropriate and expected. The current design allows for direct unit testing of the core logic without the need for such constructs.

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