Conversation
There was a problem hiding this comment.
Pull request overview
Adds a detailed implementation plan document for introducing a new SteinerTree problem model (and associated tests/CLI/docs updates) to the codebase.
Changes:
- Adds an implementation plan covering the
SteinerTreemodel structure, validity checking, and trait impls - Outlines unit tests, CLI integration points, and documentation regeneration steps
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # SteinerTree Model Implementation Plan | ||
|
|
||
| > **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. | ||
|
|
||
| **Goal:** Add the SteinerTree problem model — a minimization problem that finds a minimum-weight subtree connecting a set of terminal vertices. | ||
|
|
||
| **Architecture:** Edge-based binary optimization on graphs, following the TravelingSalesman pattern. Each edge variable is 0/1 (include or exclude). Feasibility requires selected edges to form an acyclic connected subgraph spanning all terminals. Uses `edge_weights: Vec<W>` plus a `terminals: Vec<usize>` field. |
There was a problem hiding this comment.
The PR title/description says the SteinerTree model + CLI support are added, but this PR currently only introduces an implementation plan document. Either update the PR metadata to reflect that this is documentation/planning only, or include the actual code/test/CLI/schema changes described in the plan before merging (so it truly fixes #122).
| // Count vertices in the connected component containing terminals | ||
| let component_vertices: usize = visited.iter().filter(|&&v| v && involved.iter().enumerate().any(|(i, &inv)| inv && i == 0).is_none() || v).count(); |
There was a problem hiding this comment.
In the is_valid_steiner_tree snippet, component_vertices is computed with a very complex expression that appears incorrect and wouldn’t compile as Rust (and the variable is unused). Since this is meant to be a copy/paste starting point, consider removing this line entirely or replacing it with the simpler reachable_involved count described immediately below to avoid confusion.
| // Count vertices in the connected component containing terminals | |
| let component_vertices: usize = visited.iter().filter(|&&v| v && involved.iter().enumerate().any(|(i, &inv)| inv && i == 0).is_none() || v).count(); |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #192 +/- ##
==========================================
+ Coverage 96.88% 96.90% +0.01%
==========================================
Files 200 202 +2
Lines 27537 27741 +204
==========================================
+ Hits 26680 26882 +202
- Misses 857 859 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Add SteinerTree<G, W> problem model (edge-based minimization) - Register in graph/mod.rs, prelude, CLI dispatch/aliases/create - Add --terminals CLI flag for pred create SteinerTree - 12 unit tests: creation, evaluation, brute-force, serialization - Add paper entry with Dreyfus-Wagner complexity and example figure - Regenerate problem_schemas.json and reduction_graph.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implementation SummaryChanges
Deviations from Plan
Open Questions
|
- Add SteinerTree to CLI help table in cli.rs - Add SteinerTree to random generation supported list in create.rs - Add SteinerTree to models/mod.rs re-export - Add SteinerTree to graph/mod.rs doc comment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Wrong config length (line 138) - Disconnected non-terminal edges (lines 181-182) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
--terminalsflag)Fixes #122