Fix #298: [Model] LengthBoundedDisjointPaths#659
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #659 +/- ##
==========================================
+ Coverage 96.94% 96.96% +0.02%
==========================================
Files 273 275 +2
Lines 36505 36843 +338
==========================================
+ Hits 35388 35724 +336
- Misses 1117 1119 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
Implementation SummaryChanges
Deviations from Plan
Open Questions
|
There was a problem hiding this comment.
Pull request overview
Adds a new graph decision problem model, LengthBoundedDisjointPaths, and wires it through the library exports, CLI creation flow, examples/fixtures, and docs so it can be created/serialized/tested like the existing graph models.
Changes:
- Introduces
LengthBoundedDisjointPathsmodel (schema entry, variant declaration, evaluation logic) plus unit tests and example-db spec/fixture. - Exposes the new model via
models/prelude, updates trait-consistency tests, and updates generated docs JSON + paper. - Extends
pred create(including random generation) and CLI flag documentation to support the new model.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/unit_tests/trait_consistency.rs | Adds trait-consistency coverage for the new model |
| src/unit_tests/models/graph/length_bounded_disjoint_paths.rs | New unit tests for evaluation/solver/serde behavior |
| src/unit_tests/export.rs | Minor formatting change in export test |
| src/unit_tests/example_db.rs | Minor fixture-verification formatting/refactor |
| src/models/mod.rs | Re-exports the new graph model from the top-level models module |
| src/models/graph/mod.rs | Registers the new model module + example-db specs hook |
| src/models/graph/length_bounded_disjoint_paths.rs | New model implementation, schema entry, examples, variants, and tests hook |
| src/lib.rs | Adds the new model to the public prelude exports |
| src/example_db/fixtures/examples.json | Adds a canonical example fixture for the new model |
| problemreductions-cli/src/commands/create.rs | Adds CLI creation/random-generation support and help example for the new model |
| problemreductions-cli/src/cli.rs | Adds new flags and documents required flags for the model |
| docs/src/reductions/reduction_graph.json | Adds the model to the generated reduction graph docs |
| docs/src/reductions/problem_schemas.json | Adds the model schema to generated schema docs |
| docs/paper/reductions.typ | Adds a paper section/example for the new model |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "HamiltonianPath" => "--graph 0-1,1-2,2-3", | ||
| "LengthBoundedDisjointPaths" => { | ||
| "--graph 0-1,1-6,0-2,2-3,3-6,0-4,4-5,5-6 --source 0 --sink 6 --num-paths-required 2 --bound 3" | ||
| } |
Comment on lines
+414
to
+422
| if source >= graph.num_vertices() || sink >= graph.num_vertices() { | ||
| bail!("--source and --sink must be valid graph vertices\n\n{usage}"); | ||
| } | ||
| if num_paths_required == 0 { | ||
| bail!("--num-paths-required must be positive\n\n{usage}"); | ||
| } | ||
| if max_length == 0 { | ||
| bail!("--bound must be positive\n\n{usage}"); | ||
| } |
Comment on lines
+48
to
+70
| impl<G: Graph> LengthBoundedDisjointPaths<G> { | ||
| /// Create a new Length-Bounded Disjoint Paths instance. | ||
| pub fn new( | ||
| graph: G, | ||
| source: usize, | ||
| sink: usize, | ||
| num_paths_required: usize, | ||
| max_length: usize, | ||
| ) -> Self { | ||
| assert!( | ||
| source < graph.num_vertices(), | ||
| "source must be a valid graph vertex" | ||
| ); | ||
| assert!( | ||
| sink < graph.num_vertices(), | ||
| "sink must be a valid graph vertex" | ||
| ); | ||
| assert_ne!(source, sink, "source and sink must be distinct"); | ||
| assert!( | ||
| num_paths_required > 0, | ||
| "num_paths_required must be positive" | ||
| ); | ||
| assert!(max_length > 0, "max_length must be positive"); |
Contributor
Author
|
Follow-up after review:
|
There was a problem hiding this comment.
Pull request overview
Adds a new graph decision problem model, LengthBoundedDisjointPaths, and wires it into the library, CLI creation workflow, examples/fixtures, tests, and generated documentation outputs.
Changes:
- Introduces
LengthBoundedDisjointPathsmodel (schema + variant declaration + example-db spec) and exports it viamodelsandprelude. - Adds dedicated unit tests and includes the new model in trait-consistency checks and canonical example fixtures.
- Extends the CLI
pred createflow to support buildingLengthBoundedDisjointPathsinstances (including schema-driven help flag naming).
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/models/graph/length_bounded_disjoint_paths.rs | New model implementation, schema registration, example-db spec, and variant declaration |
| src/models/graph/mod.rs | Registers/exports the new graph model and includes its example specs |
| src/models/mod.rs | Re-exports the new model from the top-level models module |
| src/lib.rs | Re-exports the new model via the public prelude |
| src/unit_tests/models/graph/length_bounded_disjoint_paths.rs | Adds model-level correctness, solver, and serialization tests |
| src/unit_tests/trait_consistency.rs | Adds trait-consistency coverage for the new model |
| src/example_db/fixtures/examples.json | Adds canonical example fixture entry for the new model |
| problemreductions-cli/src/cli.rs | Adds CLI flags list entry and new args for source/sink/path count |
| problemreductions-cli/src/commands/create.rs | Adds pred create support + schema-driven help flag naming tweaks |
| problemreductions-cli/src/problem_name.rs | Minor internal refactor in edit distance DP initialization |
| docs/src/reductions/problem_schemas.json | Generated schema docs updated to include the new model |
| docs/src/reductions/reduction_graph.json | Generated reduction graph docs updated to include the new variant node |
| docs/paper/reductions.typ | Adds paper documentation section + figure for the new model |
| src/unit_tests/export.rs | Formatting-only change in unit test assertion |
| src/unit_tests/example_db.rs | Formatting + improved labeling in fixture verification output |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+429
to
+437
| if source >= graph.num_vertices() || sink >= graph.num_vertices() { | ||
| bail!("--source and --sink must be valid graph vertices\n\n{usage}"); | ||
| } | ||
| if num_paths_required == 0 { | ||
| bail!("--num-paths-required must be positive\n\n{usage}"); | ||
| } | ||
| if max_length == 0 { | ||
| bail!("--bound must be positive\n\n{usage}"); | ||
| } |
…98-lengthboundeddisjointpaths # Conflicts: # docs/src/reductions/reduction_graph.json # src/lib.rs # src/models/mod.rs
- validate LengthBoundedDisjointPaths CLI inputs consistently - add regression coverage for constructor and CLI edge cases - document the LengthBoundedDisjointPaths CLI flow and local run path
…ADME changes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zazabap
approved these changes
Mar 16, 2026
Collaborator
zazabap
left a comment
zazabap
left a comment
There was a problem hiding this comment.
Final review passed — all CI green, coverage fixed, quality ~82%.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
zazabap
approved these changes
Mar 16, 2026
Collaborator
zazabap
left a comment
zazabap
left a comment
There was a problem hiding this comment.
Final review passed — conflicts resolved, all weaknesses fixed, quality ~82%.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add the
LengthBoundedDisjointPathsmodel, its tests, CLI creation support, canonical example, and paper entry.Fixes #298