fix: CLI UX improvements from issue #86#101
Conversation
- Swap `pred to`/`pred from` direction semantics: `pred to X` now shows
incoming neighbors (what reduces TO X), `pred from X` shows outgoing
(what X reduces FROM)
- Rename section labels: "Reduces to" → "Outgoing reductions",
"Reduces from" → "Incoming reductions"; use consistent → arrow
- Replace key-value variant display with slash notation (e.g.,
MIS/UnitDiskGraph instead of {graph=UnitDiskGraph, weight=i32})
- Switch variant resolution from positional to value-based matching
- Rename CLI flags: --edges → --graph, --bits-m → --m, --bits-n → --n,
add --edge-weights for edge-weight problems
- Add schema-driven help: `pred create MIS` (no flags) shows
problem-specific parameters, types, and examples
Closes #86
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #101 +/- ##
=======================================
Coverage 96.90% 96.90%
=======================================
Files 196 196
Lines 26811 26811
=======================================
Hits 25980 25980
Misses 831 831 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates the pred CLI UX to align with issue #86 by clarifying neighbor traversal semantics, modernizing variant/flag notation, and improving discoverability of create parameters via schema-driven help.
Changes:
- Swaps
pred to/pred fromsemantics to match “incoming to X” vs “outgoing from X”, and updates labels/arrows in graph output accordingly. - Introduces slash-based variant display/selection (e.g.,
MIS/UnitDiskGraph) and updates variant resolution behavior. - Renames several
createflags (--edges→--graph,--bits-m/n→--m/n, adds--edge-weights) and updates CLI tests/docs; adds schema-driven help when runningpred create <PROBLEM>with no data flags.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| problemreductions-cli/tests/cli_tests.rs | Updates CLI tests for new flags/labels and the swapped to/from semantics; adds coverage for schema-help behavior. |
| problemreductions-cli/src/problem_name.rs | Changes variant resolution to value-based matching for slash-notation specs. |
| problemreductions-cli/src/main.rs | Swaps traversal direction passed to neighbors() for to vs from. |
| problemreductions-cli/src/commands/reduce.rs | Changes variant formatting to slash notation. |
| problemreductions-cli/src/commands/graph.rs | Updates variant display, section labels, arrows, and tree rendering to use slash notation consistently. |
| problemreductions-cli/src/commands/create.rs | Adds schema-driven help for create with no data flags; updates parsing/usage strings for renamed flags including --graph and --edge-weights. |
| problemreductions-cli/src/cli.rs | Updates CLI help text and CreateArgs flags to the new names and adds --edge-weights. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| match matches.len() { | ||
| 1 => Ok(matches[0].clone()), | ||
| 0 => anyhow::bail!( |
There was a problem hiding this comment.
resolve_variant collects matches as references to known_variants but returns matches[0].clone(), which clones the reference rather than the map. This won’t compile (expected BTreeMap<_, _>, found &BTreeMap<_, _>). Consider collecting owned matches via .cloned() (or cloning the single match) before returning.
| && args.k.is_none() | ||
| && args.target.is_none() | ||
| && args.m.is_none() | ||
| && args.n.is_none() |
There was a problem hiding this comment.
all_data_flags_empty ignores the random-generation flags (--num-vertices, --edge-prob, --seed). As a result, pred create <PROBLEM> --num-vertices ... (without --random) is treated as “no flags” and prints schema help with exit code 0, hiding a user error. Include those flags in the emptiness check or explicitly detect “random flags without --random” and return a clear error.
| && args.n.is_none() | |
| && args.n.is_none() | |
| && args.num_vertices.is_none() | |
| && args.edge_prob.is_none() | |
| && args.seed.is_none() |
| fn print_problem_help(canonical: &str) -> Result<()> { | ||
| let schemas = collect_schemas(); | ||
| let schema = schemas.iter().find(|s| s.name == canonical); | ||
|
|
||
| if let Some(s) = schema { | ||
| eprintln!("{}\n {}\n", canonical, s.description); | ||
| eprintln!("Parameters:"); | ||
| for field in &s.fields { | ||
| let hint = type_format_hint(&field.type_name); | ||
| eprintln!( | ||
| " --{:<16} {} ({})", | ||
| field.name.replace('_', "-"), | ||
| field.description, | ||
| hint | ||
| ); | ||
| } |
There was a problem hiding this comment.
print_problem_help prints schema field names as --<field> flags, but schema fields don’t always correspond to CLI flags (e.g., SpinGlass schema has couplings/fields, while the CLI accepts --edge-weights and doesn’t accept --fields; KColoring schema lists only graph but the CLI requires --k). This makes the new help output misleading. Consider adding a per-problem mapping from schema fields/variant params to the actual CLI flags (or generating this help from CreateArgs/clap metadata plus schema descriptions).
| fn format_variant(v: &BTreeMap<String, String>) -> String { | ||
| if v.is_empty() { | ||
| "(default)".to_string() | ||
| String::new() | ||
| } else { | ||
| let pairs: Vec<String> = v.iter().map(|(k, val)| format!("{k}={val}")).collect(); | ||
| format!("{{{}}}", pairs.join(", ")) | ||
| let vals: Vec<&str> = v.values().map(|v| v.as_str()).collect(); | ||
| format!("/{}", vals.join("/")) | ||
| } |
There was a problem hiding this comment.
format_variant now returns an empty string for the default variant, but callers format it as a separate {} token with surrounding spaces (e.g., "{} {}"), which will produce double spaces and a less-informative error message for default variants. Either keep an explicit default marker (e.g., "(default)") or change call sites to concatenate the variant suffix without adding extra spaces when it’s empty.
… schema to CLI flags, fix double spaces - Include --num-vertices, --edge-prob, --seed in all_data_flags_empty() so `pred create MIS --num-vertices 10` (without --random) errors instead of showing help - Add schema_to_cli_flag() mapping so schema-driven help shows correct CLI flag names (e.g., SpinGlass "couplings" → --edge-weights) - Remove space before format_variant() in reduce.rs error message to avoid double spaces when variant is empty Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lass Replace schema_to_cli_flag mapping with proper per-problem CLI flags: - Add --couplings and --fields flags to CreateArgs for SpinGlass - Add parse_couplings() and parse_fields() parsers - Remove schema_to_cli_flag() workaround; schema field names now match CLI flags directly for all problems - Update SpinGlass example in after_help Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
pred to/pred fromsemantics:pred to X= what reduces TO X (incoming),pred from X= what X reduces FROM (outgoing)→arrowsMIS/UnitDiskGraphinstead of{graph=UnitDiskGraph, weight=i32}; value-based matching for variant resolution--edges→--graph,--bits-m→--m,--bits-n→--n, new--edge-weightsfor edge-weight problemspred create MIS(no flags) shows problem-specific parameters, types, and examples from the schema registryCloses #86
Test plan
pred to MIS,pred from MIS,pred show MIS,pred show KColoring,pred create MIS,pred create Factoring,pred create MIS --graph 0-1,1-2🤖 Generated with Claude Code