Conversation
Remove public re-exports of ~30 ReductionXToY structs, 6 gadget functions, BoolVar, LogicGadget, and JSON serialization types (EdgeJson, NodeJson, ReductionGraphJson) from rules/mod.rs. Users interact with reductions via the ReduceTo trait or ReductionGraph, never referencing these structs by name. Also remove unused ReductionColoringToILP type alias and suppress dead_code warnings on LogicGadget fields only read in tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change internal type re-exports (CopyLine, MappingGrid, CellState, Pattern, etc.) from pub to pub(crate) in unitdiskmapping/mod.rs. Change alpha_tensor and pathdecomposition modules to pub(crate). Add #[cfg(test)] guards on re-exports only needed by unit tests. Add #[doc(hidden)] _internal module for the export_mapping_stages example which needs these types. Keep ksg, triangular modules and GridKind/MappingResult as pub. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…es module - Change polynomial and truth_table modules from pub to pub(crate) since they are only used internally via crate:: imports. - Delete graph_types.rs and its unit test file (zero internal imports, confirmed by codebase audit; actual graph types live in topology/). - Remove truth_table integration tests from tests/suites/reductions.rs (equivalent coverage exists in src/unit_tests/truth_table.rs). - Remove doc example from TruthTable since the module is now internal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add public is_valid_solution(&self, config: &[usize]) -> bool methods to all graph problem types, delegating to existing private validation helpers. Also add cut_size method to MaxCut for computing partition cut sizes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add public is_valid_solution methods to MaximumSetPacking, MinimumSetCovering, BicliqueCover, CircuitSAT, and Factoring. Each delegates to existing validation logic. PaintShop already has count_switches and BMF is skipped per design. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change standalone validation free functions (is_independent_set, is_vertex_cover, is_clique, etc.) from pub to pub(crate) with #[cfg(test)] for test-only functions. Functions still used in non-test code (is_valid_coloring, is_hamiltonian_cycle, cut_size) remain pub(crate) without #[cfg(test)]. - Change 17 validation functions from pub to pub(crate) - Add #[cfg(test)] to 14 functions only used in tests - Remove validation function re-exports from graph/set/specialized mod.rs - Make submodules pub(crate) for crate-internal test access - Update graph_models.rs imports to use full module paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change these utility functions from pub to pub(crate) with #[cfg(test)] since they are only used in unit tests. This reduces the public API surface of the config module. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove from prelude: config utilities, registry types, variant types, ILP/optimization internals, NumericSize, and WeightElement. These items remain accessible via their full module paths. Add explicit imports to 16 examples, 2 integration test files that previously relied on the broader prelude: - ILP type: 13 examples + 1 test file - K3/K2 variant types: 7 examples + 2 test files - LinearConstraint/ObjectiveSense: 1 example + 1 test file Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (31.50%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #79 +/- ##
==========================================
- Coverage 96.28% 96.10% -0.19%
==========================================
Files 194 193 -1
Lines 26546 26567 +21
==========================================
- Hits 25561 25532 -29
- Misses 985 1035 +50 ☔ 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 significantly reduces the public API surface of the problemreductions crate to improve clarity and maintainability. The changes focus on internalizing implementation details that were unnecessarily exposed while keeping the core problem types and reduction functionality public.
Changes:
- Internalized ~30 reduction struct types and gadget functions from the
rulesmodule, requiring users to use trait-based reductions instead - Made
polynomialandtruth_tablemodulespub(crate)as they are internal implementation details - Deleted the unused
graph_typesmodule (graph type markers now intopologymodule) - Added
is_valid_solution()method to 14 problem types where validation is non-trivial - Internalized 17 validation helper functions (e.g.,
is_independent_set,is_clique) to internal module paths - Slimmed down the prelude by removing ILP-specific types, config utilities, registry metadata, and variant system types (still accessible via their respective modules)
- Added
_internalmodule inunitdiskmappingfor development tools like examples
Reviewed changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Updated prelude to remove ILP types, config utilities, variant system, and internal types; removed graph_types module |
| src/graph_types.rs | Deleted entirely - functionality moved to topology module |
| src/truth_table.rs | Made pub(crate) and removed public documentation example |
| src/config.rs | Made config_to_bits and bits_to_config internal test utilities |
| src/rules/mod.rs | Removed public exports of ~30 reduction types and JSON serialization structures |
| src/rules/graph.rs | Made JSON serialization types (ReductionGraphJson, NodeJson, EdgeJson) internal |
| src/rules/coloring_ilp.rs | Removed backwards compatibility type alias ReductionColoringToILP |
| src/rules/circuit_spinglass.rs | Added #[allow(dead_code)] to public fields used only in tests |
| src/rules/unitdiskmapping/mod.rs | Internalized implementation modules; added _internal for development tools |
| src/rules/unitdiskmapping/*.rs | Made various implementation details pub(crate) and added #[allow(dead_code)] where appropriate |
| src/models/graph/*.rs | Added is_valid_solution() methods; made validation helpers pub(crate) |
| src/models/set/*.rs | Added is_valid_solution() methods; made validation helpers pub(crate) |
| src/models/satisfiability/*.rs | Added is_valid_solution() to Satisfiability; made is_satisfying_assignment internal |
| src/models/specialized/*.rs | Added is_valid_solution() methods; made validation helpers pub(crate) |
| tests/suites/*.rs | Updated imports for ILP types and variant types; removed truth_table tests (moved to unit tests) |
| examples/*.rs | Added explicit imports for ILP types and variant types removed from prelude |
| src/unit_tests/graph_models.rs | Updated imports to use internal paths for validation functions |
| src/unit_tests/graph_types.rs | Deleted along with graph_types module |
Comments suppressed due to low confidence (1)
src/rules/unitdiskmapping/pathdecomposition.rs:418
- The documentation example has been changed from a runnable code block (
rust) to a non-runnable text block (text), but the example code still contains valid Rust code that appears intended to demonstrate usage. Since thepathwidthfunction andPathDecompositionMethodare now pub(crate), the example can't be tested in doc tests anymore. However, consider keeping this as a proper Rust code block with theignoreorno_runattribute (e.g.,rust,ignore) to maintain proper syntax highlighting and IDE support, rather than usingtext which loses these benefits.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
1 participant
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
ReductionXToYstructs, gadget functions, and JSON serialization types fromrulesmoduleunitdiskmappingimplementation details (CopyLine, MappingGrid, Pattern, etc.)polynomial,truth_tablemodulespub(crate); delete unusedgraph_typesmoduleis_valid_solution()method to all problem types (15 types) andcut_size()to MaxCutis_independent_set,is_clique, etc.)config_to_bits/bits_to_configutility functionsTest plan
make test— all 1477 unit + 91 integration + 45 doc tests passmake clippy— zero warnings with-D warningscargo docCloses #77
🤖 Generated with Claude Code