Skip to content

Fix #253: [Model] MultipleChoiceBranching#656

Open
GiggleLiu wants to merge 6 commits intomainfrom
issue-253-multiple-choice-branching
Open

Fix #253: [Model] MultipleChoiceBranching#656
GiggleLiu wants to merge 6 commits intomainfrom
issue-253-multiple-choice-branching

Conversation

@GiggleLiu
Copy link
Contributor

Summary

  • add an implementation plan for MultipleChoiceBranching
  • prepare issue [Model] MultipleChoiceBranching #253 for execution in the project pipeline
  • note that the model currently has no open companion rule issue and will remain an orphan node until one is filed

Fixes #253

@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

❌ Patch coverage is 98.80597% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.90%. Comparing base (ee5b77b) to head (f6f56b2).

Files with missing lines Patch % Lines
src/models/graph/multiple_choice_branching.rs 97.87% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #656      +/-   ##
==========================================
+ Coverage   96.88%   96.90%   +0.02%     
==========================================
  Files         269      271       +2     
  Lines       36036    36371     +335     
==========================================
+ Hits        34915    35247     +332     
- Misses       1121     1124       +3     

☔ 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.

@GiggleLiu
Copy link
Contributor Author

Implementation Summary

Changes

  • Added the new graph model MultipleChoiceBranching<i32> with partition validation, branching/acyclicity checks, registry metadata, and the canonical YES instance from issue [Model] MultipleChoiceBranching #253.
  • Added model tests covering constructor invariants, feasibility checks, brute-force solving, serialization, and the paper/example instance.
  • Wired the model into exports, trait-consistency checks, CLI pred create support (including the new --partition flag), CLI round-trip tests, checked-in canonical fixtures, and the paper entry.

Deviations from Plan

  • The example-db and paper now consume the checked-in fixture file, so I regenerated src/example_db/fixtures/examples.json instead of adding a separate in-memory builder entry.
  • The review helper is commit-range-based, so I ran full worktree verification first and then reviewed the committed implementation range.
  • pipeline_pr.py create successfully created PR Fix #253: [Model] MultipleChoiceBranching #656 earlier, but its post-create inspection step errored because it called gh pr view --repo ... without a PR selector.

Open Questions

  • No blocker for this PR, but MultipleChoiceBranching is currently an orphan model until a rule lands on top of it.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the new MultipleChoiceBranching graph model (Garey & Johnson ND11) to the core library and wires it through the example DB, docs, and CLI so it can be created/serialized/solved with existing tooling.

Changes:

  • Introduces MultipleChoiceBranching model implementation + variant/schema registration and example-db spec.
  • Adds unit tests (model correctness + trait consistency), example fixtures, and CLI create/solve round-trip tests.
  • Regenerates/updates docs artifacts (schemas + reduction graph) and paper content to include the new model.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/models/graph/multiple_choice_branching.rs New model implementation, schema entry, variants, and example-db spec hook
src/models/graph/mod.rs Registers module, re-exports type, and includes canonical example specs
src/models/mod.rs Re-exports MultipleChoiceBranching from graph models
src/lib.rs Adds MultipleChoiceBranching to the public prelude
src/unit_tests/models/graph/multiple_choice_branching.rs New unit tests covering construction, validation, evaluate(), brute-force, and serde round-trip
src/unit_tests/trait_consistency.rs Adds trait-consistency smoke test instantiation for the new model
src/example_db/fixtures/examples.json Adds canonical example instance + satisfying/optimal configs for the model
problemreductions-cli/src/cli.rs Adds --partition flag and documents MCB flags/examples in help text
problemreductions-cli/src/commands/create.rs Implements pred create MultipleChoiceBranching/... parsing, including --partition parsing helper
problemreductions-cli/tests/cli_tests.rs Adds CLI tests for create, example create, and create→solve piping
docs/src/reductions/problem_schemas.json Adds generated schema entry for MultipleChoiceBranching
docs/src/reductions/reduction_graph.json Adds node entry for MultipleChoiceBranching and regenerates graph indices
docs/paper/reductions.typ Adds paper section + figure for MultipleChoiceBranching based on the canonical fixture

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

Comment on lines +491 to +503
let threshold = args.bound.ok_or_else(|| {
anyhow::anyhow!(
"MultipleChoiceBranching requires --bound\n\n\
Usage: pred create MultipleChoiceBranching/i32 --arcs \"0>1,0>2,1>3,2>3,1>4,3>5,4>5,2>4\" --weights 3,2,4,1,2,3,1,3 --partition \"0,1;2,3;4,7;5,6\" --bound 10"
)
})? as i32;
(
ser(MultipleChoiceBranching::new(
graph,
weights,
partition,
threshold,
))?,
Comment on lines +209 to +223
let arcs = graph.arcs();
let mut in_degree = vec![0usize; graph.num_vertices()];
for (index, &selected) in config.iter().enumerate() {
if selected == 1 {
let (_, target) = arcs[index];
in_degree[target] += 1;
if in_degree[target] > 1 {
return false;
}
}
}

let selected_arcs: Vec<bool> = config.iter().map(|&selected| selected == 1).collect();
if !graph.is_acyclic_subgraph(&selected_arcs) {
return false;
Comment on lines +1473 to +1495
/// Parse `--partition` as semicolon-separated groups of comma-separated arc indices.
/// E.g., "0,1;2,3;4,7;5,6"
fn parse_partition_groups(args: &CreateArgs) -> Result<Vec<Vec<usize>>> {
let partition_str = args.partition.as_deref().ok_or_else(|| {
anyhow::anyhow!(
"MultipleChoiceBranching requires --partition (e.g., \"0,1;2,3;4,7;5,6\")"
)
})?;

partition_str
.split(';')
.map(|group| {
group.trim()
.split(',')
.map(|s| {
s.trim()
.parse::<usize>()
.map_err(|e| anyhow::anyhow!("Invalid partition index: {}", e))
})
.collect()
})
.collect()
}
@GiggleLiu
Copy link
Contributor Author

Review Pipeline Report

Check Result
Copilot comments 3 fixed
Issue/human comments checked, no additional fixes needed
Structural review initial fail fixed
CI green
Agentic test passed with notes
Needs human decision 1 item
Board Review pool -> Under review -> Final review

Remaining issues for final review

  • If pred create ... fails upstream in a pipe, downstream pred solve - can still emit a secondary EOF/JSON parse error. I did not auto-fix this because it is broader CLI pipe-error UX outside the MultipleChoiceBranching review scope. Recommended maintainer decision: track it as a separate CLI UX follow-up unless you want to explicitly expand this PR.

Generated by review-pipeline

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.

[Model] MultipleChoiceBranching

2 participants