fix: CLI QA improvements — creation, aliases, help, schemas by GiggleLiu · Pull Request #190 · CodingThrust/problem-reductions

GiggleLiu · 2026-03-07T06:32:09Z

Summary

Fixes the bugs and UX issues reported in #189:

Bug 1 (9 uncreatable types): Add CLI creation for 8 problem types (MaximalIS, BinPacking, PaintShop, MaximumSetPacking, MinimumSetCovering, BicliqueCover, BMF, CVP). ILP/CircuitSAT get clear "use reduction" messages.
Bug 2 (MaximalIS self-referential): Fixed — MaximalIS now has a proper creation handler.
Bug 4 (bare pred appends solve help): Fixed — print_subcommand_help_hint now matches first line only.
Bug 8 (MaxMatching alias): Added MaxMatching → MaximumMatching alias.
Bug 9 (exit code 0 for help): pred create <PROBLEM> with no flags now exits 2.
Bug 10 (--timeout default twice): Removed redundant [default: 0] from doc comment.
Makefile: Fixed wrong flag names (--edges → --graph, --bits-m → --m).
Schemas: Aligned BMF, PaintShop, CircuitSAT schemas to constructor params (not internal fields).
Skill: Added Step 4.5 (CLI creation support) to add-model skill.

Test plan

make check passes (fmt + clippy + test)
make cli-demo passes (all 20 steps, 26 JSON files)
All 8 new problem types tested via pred create
Bare pred no longer appends solve help
pred create MIS (no flags) exits non-zero
MaxMatching alias resolves correctly

Closes #189

🤖 Generated with Claude Code

…189) Closes #189 - Add CLI creation for 8 previously unsupported problem types: MaximalIS, BinPacking, PaintShop, MaximumSetPacking, MinimumSetCovering, BicliqueCover, BMF, ClosestVectorProblem - Add MaxMatching alias for MaximumMatching - Fix bare `pred` appending unrelated solve help text (match first line only) - Fix `pred create <PROBLEM>` exiting 0 when showing help (now exits 2) - Fix `--timeout` default shown twice in help - Fix Makefile using wrong flag names (--edges → --graph, --bits-m → --m) - Align schemas to constructor params (BMF, PaintShop, CircuitSAT) - Add CLI creation instructions to add-model skill (Step 4.5) - Add parse_comma_list and parse_edge_pairs utilities Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR addresses CLI QA issues in pred create by adding creation support for previously uncreatable problem types, improving aliasing and help behavior, and aligning selected problem schemas with constructor-facing inputs.

Changes:

Added pred create handlers + flags for multiple additional problem types (e.g., MaximalIS, BinPacking, PaintShop, set problems, BicliqueCover, BMF, CVP) and improved exit behavior when invoked without data flags.
Improved CLI UX: fixed bare pred help hint matching, added MaxMatching alias, updated demo Makefile flags, and removed duplicated --timeout default in help.
Updated schemas to reflect constructor parameters (not derived/internal fields) for PaintShop, CircuitSAT, and BMF; updated “add-model” skill docs accordingly.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/models/misc/paintshop.rs`	Schema now exposes the constructor-facing `sequence` input instead of internal derived fields.
`src/models/formula/circuit.rs`	Schema no longer lists derived `variables`; keeps only `circuit`.
`src/models/algebraic/bmf.rs`	Schema removes derived `m`/`n` and keeps `matrix` + `k` (rank).
`problemreductions-cli/src/commands/create.rs`	Adds creation handlers for multiple new problem types; changes “no flags” behavior to print schema help then exit(2).
`problemreductions-cli/src/cli.rs`	Adds new `CreateArgs` flags; fixes help-hint matching and removes redundant timeout default text.
`problemreductions-cli/src/util.rs`	Adds shared parsing helpers for comma lists and edge pairs.
`problemreductions-cli/src/problem_name.rs`	Adds `MaxMatching -> MaximumMatching` alias resolution.
`problemreductions-cli/src/commands/graph.rs`	Removes stray whitespace line.
`problemreductions-cli/tests/cli_tests.rs`	Formatting-only adjustments to assertions/command invocation.
`Makefile`	Updates CLI demo to use correct flag names (`--graph`, `--m`, `--n`).
`.claude/skills/add-model/SKILL.md`	Documents new step for adding CLI creation support and schema alignment guidance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-07T06:37:37Z

problemreductions-cli/src/commands/create.rs

            None
        };
-        return print_problem_help(canonical, gt);
+        print_problem_help(canonical, gt)?;


print_problem_help() renders flag names directly from the model schema field names (with _ → -). For several newly creatable problems, the schema field names do not match the actual CLI flags (e.g., MinimumSetCovering uses universe_size in the schema but the CLI uses --universe; BMF schema uses k but the CLI expects --rank; CVP schema uses target/bounds but the CLI expects --target-vec and hard-codes bounds). This makes pred create <PROBLEM> (no flags) output misleading. Suggest adding a canonical-aware mapping in print_problem_help() (similar to the existing geometry graph→positions mapping) so the help shows the real flags, or aligning the CLI flag names with schema fields where feasible.

Suggested change

print_problem_help(canonical, gt)?;

print_problem_help(canonical, gt)?;

// Some problems use CLI flag names that differ from their schema field names.

// Add clarifying notes so that "pred create <PROBLEM>" shows the actual flags.

match canonical.as_str() {

// MinimumSetCovering schema uses `universe_size`, but the CLI flag is `--universe`.

"MinimumSetCovering" => {

eprintln!();

eprintln!(

"Note: for MinimumSetCovering, use the CLI flag `--universe` (not `--universe-size`)."

);

}

// BMF schema uses `k`, but the CLI flag is `--rank`.

"BMF" => {

eprintln!();

eprintln!(

"Note: for BMF, use the CLI flag `--rank` (this corresponds to schema field `k`)."

);

}

// ClosestVectorProblem schema uses `target`/`bounds`, but the CLI expects `--target-vec`

// and currently hard-codes bounds.

"ClosestVectorProblem" => {

eprintln!();

eprintln!(

"Note: for ClosestVectorProblem, use the CLI flag `--target-vec` for the target."

);

eprintln!(" Bounds are currently fixed in the CLI and not configurable via flags.");

}

_ => {}

}

Copilot · 2026-03-07T06:37:38Z

problemreductions-cli/src/commands/create.rs

+                    "CVP requires --basis, --target-vec\n\n\
+                     Usage: pred create CVP --basis \"1,0;0,1\" --target-vec \"0.5,0.5\""
+                )
+            })?;
+            let target_str = args
+                .target_vec
+                .as_deref()
+                .ok_or_else(|| anyhow::anyhow!("CVP requires --target-vec (e.g., \"0.5,0.5\")"))?;
+            let basis: Vec<Vec<i32>> = basis_str
+                .split(';')
+                .map(|row| util::parse_comma_list(row.trim()))
+                .collect::<Result<Vec<_>>>()?;
+            let target: Vec<f64> = util::parse_comma_list(target_str)?;


CVP creation hard-codes coefficient bounds to [-10, 10] with no CLI override, but the underlying schema includes a bounds parameter. This makes the created instance potentially incorrect for many inputs and also conflicts with the schema-driven help output. Consider exposing bounds as a flag (e.g., --bounds -10:10 or per-variable bounds) or updating the schema/help path to clearly document that bounds are fixed/defaulted in the CLI.

Suggested change

"CVP requires --basis, --target-vec\n\n\

Usage: pred create CVP --basis \"1,0;0,1\" --target-vec \"0.5,0.5\""

)

})?;

let target_str = args

.target_vec

.as_deref()

.ok_or_else(|| anyhow::anyhow!("CVP requires --target-vec (e.g., \"0.5,0.5\")"))?;

let basis: Vec<Vec<i32>> = basis_str

.split(';')

.map(|row| util::parse_comma_list(row.trim()))

.collect::<Result<Vec<_>>>()?;

let target: Vec<f64> = util::parse_comma_list(target_str)?;

"CVP requires --basis and --target-vec\n\n\

Usage: pred create CVP --basis \"1,0;0,1\" --target-vec \"0.5,0.5\"\n\n\

Note: CLI-created CVP instances use fixed coefficient bounds [-10, 10]. \

For custom bounds, construct the instance via JSON or the Rust API."

)

})?;

let target_str = args

.target_vec

.as_deref()

.ok_or_else(|| {

anyhow::anyhow!(

"CVP requires --target-vec (e.g., \"0.5,0.5\"). \

CLI-created CVP instances use fixed coefficient bounds [-10, 10]; \

for custom bounds, construct the instance via JSON or the Rust API."

)

})?;

let basis: Vec<Vec<i32>> = basis_str

.split(';')

.map(|row| util::parse_comma_list(row.trim()))

.collect::<Result<Vec<_>>>()?;

let target: Vec<f64> = util::parse_comma_list(target_str)?;

// CLI currently fixes coefficient bounds to [-10, 10]; for custom bounds use JSON/Rust API.

Copilot · 2026-03-07T06:37:38Z

problemreductions-cli/src/util.rs

+            let u: usize = parts[0].parse()?;
+            let v: usize = parts[1].parse()?;


parse_edge_pairs() trims the whole u-v pair but does not trim each side before parsing. Inputs like "0- 1" or "0 -1" will fail with a generic parse error even though they’re easy to accept. Trimming parts[0] / parts[1] (and mapping parse errors to a clearer "Invalid edge" message) would make this parser more robust and consistent with parse_positions() which trims each component.

Suggested change

let u: usize = parts[0].parse()?;

let v: usize = parts[1].parse()?;

let u_str = parts[0].trim();

let v_str = parts[1].trim();

let u: usize = u_str.parse().map_err(|e| {

anyhow::anyhow!(

"Invalid edge '{}': could not parse vertex '{}': {e}",

pair.trim(),

u_str

)

})?;

let v: usize = v_str.parse().map_err(|e| {

anyhow::anyhow!(

"Invalid edge '{}': could not parse vertex '{}': {e}",

pair.trim(),

v_str

)

})?;

Copilot · 2026-03-07T06:37:38Z

problemreductions-cli/src/commands/create.rs

    // Show schema-driven help when no data flags are provided
    if all_data_flags_empty(args) {
        let gt = if graph_type != "SimpleGraph" {
            Some(graph_type)
        } else {


The early "no data flags" help path runs before the main match, so pred create ILP / pred create CircuitSAT (with no flags) will print schema help + exit(2) instead of showing the intended "via reduction" guidance from the later "ILP" | "CircuitSAT" match arm. Consider special-casing these canonical names before this block (or incorporating the reduction-only message into the help path) so the UX matches the PR description.

…g, edge trim - Update 3 tests to expect non-zero exit when showing help (issue #189 item 9) - Move ILP/CircuitSAT check before empty-flags help so they get the "via reduction" message instead of generic schema help - Trim each side of edge pairs in parse_edge_pairs for robustness Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

codecov · 2026-03-07T06:52:30Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.88%. Comparing base (92cdd0a) to head (2c7f70d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #190   +/-   ##
=======================================
  Coverage   96.88%   96.88%           
=======================================
  Files         200      200           
  Lines       27537    27537           
=======================================
  Hits        26680    26680           
  Misses        857      857

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GiggleLiu requested a review from Copilot March 7, 2026 06:32

Copilot started reviewing on behalf of GiggleLiu March 7, 2026 06:32 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

fix: add --bounds flag for CVP creation, consistent with schema

2c7f70d

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GiggleLiu merged commit 5fbde0e into main Mar 7, 2026
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: CLI QA improvements — creation, aliases, help, schemas#190

fix: CLI QA improvements — creation, aliases, help, schemas#190
GiggleLiu merged 3 commits intomainfrom
fix/cli-qa-189

GiggleLiu commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

codecov bot commented Mar 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        print_problem_help(canonical, gt)?;
+        print_problem_help(canonical, gt)?;
+        // Some problems use CLI flag names that differ from their schema field names.
+        // Add clarifying notes so that "pred create <PROBLEM>" shows the actual flags.
+        match canonical.as_str() {
+            // MinimumSetCovering schema uses `universe_size`, but the CLI flag is `--universe`.
+            "MinimumSetCovering" => {
+                eprintln!();
+                eprintln!(
+                    "Note: for MinimumSetCovering, use the CLI flag `--universe` (not `--universe-size`)."
+                );
+            }
+            // BMF schema uses `k`, but the CLI flag is `--rank`.
+            "BMF" => {
+                eprintln!();
+                eprintln!(
+                    "Note: for BMF, use the CLI flag `--rank` (this corresponds to schema field `k`)."
+                );
+            }
+            // ClosestVectorProblem schema uses `target`/`bounds`, but the CLI expects `--target-vec`
+            // and currently hard-codes bounds.
+            "ClosestVectorProblem" => {
+                eprintln!();
+                eprintln!(
+                    "Note: for ClosestVectorProblem, use the CLI flag `--target-vec` for the target."
+                );
+                eprintln!("      Bounds are currently fixed in the CLI and not configurable via flags.");
+            }
+            _ => {}
+        }

		let u: usize = parts[0].parse()?;
		let v: usize = parts[1].parse()?;

-            let u: usize = parts[0].parse()?;
-            let v: usize = parts[1].parse()?;
+            let u_str = parts[0].trim();
+            let v_str = parts[1].trim();
+            let u: usize = u_str.parse().map_err(|e| {
+                anyhow::anyhow!(
+                    "Invalid edge '{}': could not parse vertex '{}': {e}",
+                    pair.trim(),
+                    u_str
+                )
+            })?;
+            let v: usize = v_str.parse().map_err(|e| {
+                anyhow::anyhow!(
+                    "Invalid edge '{}': could not parse vertex '{}': {e}",
+                    pair.trim(),
+                    v_str
+                )
+            })?;

Conversation

GiggleLiu commented Mar 7, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Mar 7, 2026 •

edited

Loading