feat(groq): A whimsical CLI tool that selects the optimal set of items to fit within a weight capacity using a simple knapsack algorithm.

[polsala]

Implementation Summary

• Utility: nightly-survival-packer
• Provider: groq
• Location: node-utils/nightly-nightly-survival-packer
• Files Created: 3
• Description: A whimsical CLI tool that selects the optimal set of items to fit within a weight capacity using a simple knapsack algorithm.

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 node-utils/nightly-nightly-survival-packer.
• 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 node-utils/nightly-nightly-survival-packer/README.md
• Run tests located in node-utils/nightly-nightly-survival-packer/tests/

Links

• Generated docs and examples committed alongside this change.

Mock Justification

• Not applicable; generator did not introduce new mocks.

[polsala]

🤖 Review by GROQ Agent

✅ What's solid

• Self‑contained utility – All source, tests, and documentation live under node-utils/nightly-nightly-survival-packer; no cross‑module side‑effects.
• Clear entry point – The if (require.main === module) { main(); } guard makes the file usable both as a CLI and as an importable module.
• Straight‑forward algorithm – The brute‑force bit‑mask approach is easy to read and works perfectly for the tiny item sets typical of a whimsical demo.
• Basic test coverage – The three end‑to‑end tests exercise the most common success paths (normal capacity, exact match, empty list).

---

🧪 Tests

Observation	Recommendation
Tests run the CLI via spawnSync, which is fine for integration testing, but they don’t exercise the pure functions (parseArgs, parseItems, knapsack).	Export those helpers from src/main.js and add unit tests that call them directly. This speeds up the suite and gives finer‑grained failure diagnostics.
Edge‑case handling isn’t verified (e.g., missing --capacity, non‑numeric weight, negative capacity, duplicate item names, very large item sets).	Add a few targeted tests, for example: js\ntest('rejects non‑numeric capacity', () => {\n const out = run(['--items', 'water:3', '--capacity', 'foo']);\n expect(out).toMatch(/invalid capacity/i);\n});\n
The current tests only check the happy path JSON output.	Include a test that ensures the program always prints valid JSON (even on error) and that the process exits with a non‑zero code on malformed input.
Performance isn’t measured, but the brute‑force algorithm will explode for >20 items.	Consider a benchmark test that runs the CLI with, say, 15 items and asserts it finishes under a reasonable threshold (e.g., 200 ms). This will surface the exponential blow‑up early.

---

🔒 Security

• Input validation – parseItems blindly trusts the name:weight format. Supplying water:abc yields weight: NaN, which then propagates into the knapsack loop and can cause the comparison w <= capacity to always be false, resulting in an empty selection without any warning.
  Fix: Validate that weightStr parses to a finite integer and reject otherwise with a clear error message.

  const weight = Number(weightStr);
  if (!Number.isInteger(weight) || weight < 0) {
    throw new Error(`Invalid weight for item "${name}": "${weightStr}"`);
  }

• Bit‑mask overflow – The expression 1 << n works only for n < 31 (JavaScript bitwise operators use 32‑bit signed ints). With 31+ items the algorithm silently wraps, producing incorrect results.
  Fix: Guard against large n and either fall back to a DP solution or emit a helpful error:

  if (n > 30) {
    throw new Error('Too many items for brute‑force knapsack (max 30).');
  }

• Child process usage in tests – spawnSync inherits the parent’s environment. If the test runner ever runs with elevated privileges, a maliciously crafted argument could be executed. While unlikely in CI, it’s good practice to sanitize arguments before passing them to spawnSync. Consider using a wrapper that validates the argument array.

Overall, the utility doesn’t touch external resources, but adding the above validation steps will make it robust against malformed or malicious CLI input.

---

🧩 Docs / Developer Experience

• README formatting – The current file is a single paragraph with embedded \n literals. Render it as proper Markdown with headings, code fences, and bullet points for options.

  # Nightly Survival Packer
  
  A whimsical CLI utility that helps post‑apocalyptic survivors decide what to pack in their limited‑capacity backpack. It uses a simple knapsack algorithm to maximize total weight without exceeding the capacity.
  
  ## Installation
  
  ```sh
  npm install --save-dev ./node-utils/nightly-nightly-survival-packer

  Usage

  node src/main.js --items "water:3,food:2,radio:5" --capacity 5

  Output:

  ["water","food"]

• Error handling documentation – The README should describe what happens on invalid input (e.g., “prints an error message to stderr and exits with code 1”). This sets correct expectations for users.

• Algorithm limits – Mention the exponential nature of the current implementation and the practical limit on the number of items (e.g., ≤ 20) to avoid surprising users with long runtimes.

• Contribution guide – Since the utility is generated by Groq, a short note on how to regenerate or extend it would be helpful for future maintainers.

---

🧱 Mocks / Fakes

• The PR notes “Not applicable; generator did not introduce new mocks.” While true for the production code, the test suite could benefit from function‑level mocking:
  • Mock process.argv instead of spawning a child process when testing parseArgs. This would let you unit‑test argument parsing without the overhead of a subprocess.
  • Mock console.log to capture output directly in a unit test of main():

    test('main prints selected items', () => {
      const logSpy = jest.spyOn(console, 'log').mockImplementation(() => {});
      // set process.argv manually
      process.argv = ['node', 'main.js', '--items', 'a:1,b:2', '--capacity', '2'];
      require('../src/main'); // triggers main()
      expect(logSpy).toHaveBeenCalledWith(JSON.stringify(['b']));
      logSpy.mockRestore();
    });

  • These lighter‑weight tests run faster and make failures easier to diagnose, while the existing end‑to‑end tests still provide full‑stack coverage.

---

TL;DR Action items

1. Add input validation for capacity and item weights; guard against >30 items.
2. Export helper functions (parseArgs, parseItems, knapsack) and write unit tests for them.
3. Expand test matrix to cover malformed arguments, negative capacities, duplicate names, and performance limits.
4. Polish the README: proper Markdown, installation steps, error handling, algorithm limits, and contribution notes.
5. Consider lightweight mocks for process.argv and console.log to complement the existing CLI‑level tests.

These improvements will make the utility safer, more maintainable, and friendlier for both end‑users and future contributors.

[polsala]

🤖 Review by GEMINI Agent

✅ What's solid

• Clear Problem Domain: The utility's purpose (knapsack for survival packing) is well-defined and immediately understandable from the name and description.
• Self-Contained Utility: The PR introduces a new, isolated utility within node-utils, adhering to the "additive and self-contained" principle. This minimizes potential side effects on existing codebase.
• Direct CLI Execution: The use of if (require.main === module) for the main() function is a standard and robust pattern for Node.js CLI tools.
• Correct Knapsack Logic: The knapsack function correctly implements a brute-force 0/1 knapsack algorithm using bitmasks, which is suitable for small input sizes.
• Basic Argument Parsing: The parseArgs and parseItems functions handle the expected input formats for items and capacity.

🧪 Tests

• CLI Integration Testing: The use of child_process.spawnSync to execute the CLI tool as a separate process is an excellent approach for integration testing, ensuring the entire command-line interface works as expected.
• Basic Functional Coverage: The existing tests cover a few common scenarios, including selecting items within capacity and handling an empty item list.

Actionable Feedback:

• Error Handling Tests: Introduce tests for invalid or malformed inputs to ensure the CLI handles them gracefully (e.g., prints an Error: message to stderr and exits with a non-zero code).
  • Test cases for --items with incorrect format (e.g., water, food:abc).
  • Test cases for --capacity with non-integer or negative values.
  • Test cases for missing values after --items or --capacity (e.g., node src/main.js --items --capacity 5).
• Edge Case Coverage:
  • Add a test where no items fit within the given capacity (expected output: []).
  • Test with capacity 0.
  • Test with items having 0 weight.
  • Test with a single item.
• Performance/Scale Testing: While the current knapsack algorithm is correct for small n, its O(2^n) complexity means it will become very slow for a larger number of items. Consider adding a test with a moderately large number of items (e.g., 20-25) to demonstrate its current performance characteristics or to highlight potential limitations.

🔒 Security

• Input Validation: The current implementation lacks robust input validation for both --items and --capacity. Malformed inputs could lead to unexpected behavior or crashes.
  • Capacity: parseInt will return NaN for non-numeric input, which will then be treated as 0 in comparisons (w <= capacity). This might lead to unexpected results (e.g., knapsack always returning [] if capacity is NaN).
  • Items: parseInt(weightStr, 10) for item weights also returns NaN for non-numeric input. An item with NaN weight will cause w += items[i].weight to propagate NaN, potentially leading to incorrect bestWeight calculations.
• Denial of Service (DoS) Vector: The knapsack algorithm's O(2^n) complexity can be exploited. If a user provides a very large number of items (e.g., n > 30), the process could consume excessive CPU time, leading to a DoS.

Actionable Feedback:

• Validate Capacity: Before passing capacity to knapsack, ensure it is a non-negative integer. If not, print an error to stderr and exit with a non-zero status code.

  function parseArgs() {
    const args = process.argv.slice(2);
    const result = { items: "", capacity: 0 };
    for (let i = 0; i < args.length; i++) {
      if (args[i] === "--items") {
        if (i + 1 >= args.length) {
          console.error("Error: --items requires a value.");
          process.exit(1);
        }
        result.items = args[++i];
      } else if (args[i] === "--capacity") {
        if (i + 1 >= args.length) {
          console.error("Error: --capacity requires a value.");
          process.exit(1);
        }
        const parsedCapacity = parseInt(args[++i], 10);
        if (isNaN(parsedCapacity) || parsedCapacity < 0) {
          console.error("Error: Capacity must be a non-negative integer.");
          process.exit(1);
        }
        result.capacity = parsedCapacity;
      }
    }
    return result;
  }

• Validate Item Weights: In parseItems, ensure that weightStr can be successfully parsed into a positive integer. If not, reject the item or exit with an error.

  function parseItems(str) {
    if (!str) return [];
    return str.split(",").filter(Boolean).map(pair => {
      const parts = pair.split(":");
      if (parts.length !== 2) {
        console.error(`Error: Invalid item format: "${pair}". Items must be "name:weight".`);
        process.exit(1);
      }
      const [name, weightStr] = parts;
      const weight = parseInt(weightStr, 10);
      if (!name.trim() || isNaN(weight) || weight < 0) {
        console.error(`Error: Invalid item name or weight: "${pair}". Weight must be a non-negative integer.`);
        process.exit(1);
      }
      return { name: name.trim(), weight: weight };
    });
  }

• Implement Item Limit: Add a check in main or knapsack to limit the number of items.

  function knapsack(items, capacity) {
    const n = items.length;
    const MAX_ITEMS = 20; // Example limit, adjust based on acceptable performance
    if (n > MAX_ITEMS) {
      console.error(`Error: Too many items (${n}). Maximum allowed is ${MAX_ITEMS} for performance reasons.`);
      process.exit(1);
    }
    // ... existing knapsack logic ...
  }

🧩 Docs/DX

• README Clarity: The README.md provides a good overview, usage instructions, and examples. The "whimsical" tone aligns with the utility's description.
• Code Comments: The main.js file includes a helpful top-level comment explaining its purpose.

Actionable Feedback:

• Error Handling Documentation: Update the README.md to describe how the CLI behaves when invalid inputs are provided (e.g., non-numeric capacity, malformed item strings, too many items). This will improve the developer experience by setting clear expectations.
• Performance Caveats: Given the O(2^n) complexity of the knapsack algorithm, it would be beneficial to add a note in the README.md about the practical limits on the number of items that can be processed efficiently. This manages user expectations and prevents frustration.
  • Example: "Note: Due to the nature of the algorithm, performance may degrade significantly with more than ~20-25 items."
• Dependency Management: For Node.js utilities, it's common practice to include a package.json file, even for a single script. This allows for defining metadata, scripts, and potential future dependencies. While not strictly necessary for this self-contained script, it's good practice for discoverability and future expansion.
  • Consider adding a package.json with name, version, description, and main fields.

🧱 Mocks/Fakes

• No New Mocks: The PR body states "Not applicable; generator did not introduce new mocks," which is accurate as no new mocking infrastructure or patterns are introduced.
• Subprocess for CLI Testing: The test suite effectively uses child_process.spawnSync to run the actual CLI, which is a form of integration testing rather than mocking the CLI itself. The comment // Mock rationale: we run the CLI in a subprocess to capture its stdout without external dependencies. clarifies this approach.

Actionable Feedback:

• Clarify "Mock Rationale": The comment // Mock rationale: we run the CLI in a subprocess to capture its stdout without external dependencies. is slightly misleading. While it explains why spawnSync is used, it's not a "mock rationale" in the traditional sense of mocking internal dependencies. It's an integration test strategy. Consider rephrasing for clarity, perhaps to "Integration Test Strategy" or simply "Test Strategy".
  • Example: // Test strategy: The CLI is run in a subprocess to capture its stdout, ensuring end-to-end functionality without mocking internal components.