Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #595 +/- ##
==========================================
+ Coverage 96.49% 96.51% +0.02%
==========================================
Files 204 206 +2
Lines 28397 28614 +217
==========================================
+ Hits 27401 27617 +216
- Misses 996 997 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add Minesweeper as a SatisfactionProblem in src/models/misc/. The problem determines if a partially revealed Minesweeper grid has a consistent mine assignment satisfying all revealed cell constraints (NP-complete). - Model with evaluate(), serialization, and declare_variants! - CLI support: create, dispatch, alias resolution - 8 unit tests covering satisfiable/unsatisfiable instances and solver - Example: minesweeper_consistency.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add config.len() validation in evaluate() for defensive programming - Register minesweeper_consistency example in tests/suites/examples.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Validate mine count <= 8 (physical maximum in Minesweeper) - Detect duplicate positions within revealed/unrevealed lists - Detect overlap between revealed and unrevealed positions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implementation SummaryChanges
Deviations from Plan
Open Questions
|
There was a problem hiding this comment.
Pull request overview
Adds a new “Minesweeper Consistency” satisfaction problem model to the problemreductions library, wires it into the public exports and CLI, and provides an example + tests to exercise the new model end-to-end.
Changes:
- Introduce
Minesweeperas a newSatisfactionProblemmodel (schema registration, variants, evaluation logic) plus unit tests. - Integrate
Minesweeperinto the CLI (name aliasing, JSON dispatch load/serialize,pred createflags) and document flags. - Add a runnable example and include it in the examples test suite.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/suites/examples.rs | Adds the new Minesweeper example to the examples test harness. |
| src/unit_tests/models/misc/minesweeper.rs | New unit tests covering construction, evaluation, serialization, and brute-force solving. |
| src/models/mod.rs | Re-exports Minesweeper at the models module level. |
| src/models/misc/mod.rs | Registers the new misc model module and re-export. |
| src/models/misc/minesweeper.rs | Implements the Minesweeper problem model and registers schema/variants. |
| src/lib.rs | Exposes Minesweeper via the crate prelude. |
| problemreductions-cli/src/problem_name.rs | Adds minesweeper alias resolution to canonical Minesweeper. |
| problemreductions-cli/src/dispatch.rs | Enables CLI JSON load/serialize support for Minesweeper. |
| problemreductions-cli/src/commands/create.rs | Adds CLI instance creation for Minesweeper and parsing for --revealed. |
| problemreductions-cli/src/cli.rs | Documents Minesweeper flags and adds --rows/--cols/--revealed to CLI args. |
| examples/minesweeper_consistency.rs | New example demonstrating satisfiability solving for a classic pattern. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/models/misc/minesweeper.rs
Outdated
| // Build position -> index map for unrevealed cells | ||
| let pos_to_idx: HashMap<(usize, usize), usize> = self | ||
| .unrevealed | ||
| .iter() | ||
| .enumerate() | ||
| .map(|(i, &(r, c))| ((r, c), i)) | ||
| .collect(); |
There was a problem hiding this comment.
evaluate() rebuilds a HashMap from position→index on every call. This is particularly expensive with BruteForce, which may call evaluate() O(2^n) times, turning the solve into O(2^n * n) extra hashing work. Consider precomputing/caching the position→index mapping (or per-revealed-cell neighbor index lists) once during construction (and reconstructing after deserialization if needed) so that evaluate() only does cheap indexed lookups.
| // Minesweeper | ||
| "Minesweeper" => { | ||
| let rows = args.rows.ok_or_else(|| { | ||
| anyhow::anyhow!( | ||
| "Minesweeper requires --rows, --cols, and --revealed\n\n\ | ||
| Usage: pred create Minesweeper --rows 3 --cols 3 --revealed \"1,1,1\"" | ||
| ) | ||
| })?; | ||
| let cols = args | ||
| .cols | ||
| .ok_or_else(|| anyhow::anyhow!("Minesweeper requires --cols"))?; | ||
| let revealed = parse_revealed(args)?; | ||
| let revealed_positions: std::collections::HashSet<(usize, usize)> = | ||
| revealed.iter().map(|&(r, c, _)| (r, c)).collect(); | ||
| let unrevealed: Vec<(usize, usize)> = (0..rows) | ||
| .flat_map(|r| (0..cols).map(move |c| (r, c))) | ||
| .filter(|pos| !revealed_positions.contains(pos)) | ||
| .collect(); | ||
| ( | ||
| ser(Minesweeper::new(rows, cols, revealed, unrevealed))?, | ||
| resolved_variant.clone(), | ||
| ) |
There was a problem hiding this comment.
The CLI create path relies on Minesweeper::new() assertions for validation (bounds, duplicates, overlaps, count<=8). If a user passes invalid --revealed coordinates or duplicates, the CLI will panic instead of returning a structured error. Prefer validating revealed in the CLI (bounds check against rows/cols, duplicate positions) and returning a bail!(...)/anyhow!(...) error before calling Minesweeper::new().
Precompute neighbor indices at construction time (and rebuild on deserialization via serde `from`) so evaluate() uses cheap indexed lookups instead of rebuilding a HashMap on every call. Add CLI-side validation for revealed cell bounds, count limits, and duplicates so invalid input returns a structured error instead of panicking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
SatisfactionProbleminsrc/models/misc/pred create Minesweeper), unit tests, and example programFixes #135