refactor: variant-level reduction graph with path-based API by GiggleLiu · Pull Request #72 · CodingThrust/problem-reductions

GiggleLiu · 2026-02-15T15:35:17Z

Summary

Refactor reduction graph to use variant-level nodes: each node is a unique (problem_name, variant) pair, with edges exclusively from #[reduction] registrations
Replace natural/inferred edges with explicit cast reductions (e.g., MIS<UnitDiskGraph> → MIS<SimpleGraph>) registered via #[reduction] macros in *_casts.rs files
Add DynReductionResult trait and store reduce_fn on graph edges for type-erased execution
Add ExecutablePath<S, T> and ChainedReduction<S, T> for typed multi-step reduction execution
Add ReductionPath (data: names + variants per step) with make_executable() to convert to ExecutablePath
Unify path finding into a shared Dijkstra core with lookup_node() for first-class node indexing
Make find_cheapest_path variant-aware (takes exact (name, variant_map) pairs)
Improve downcast panic messages with expected/actual type names
Update docs (design.md, getting-started.md) and reduction graph visualization

Test plan

make test — all 1447 tests pass
make clippy — no warnings
Variant cast paths verified (e.g., MIS<UnitDiskGraph> → MIS<SimpleGraph> → MVC<SimpleGraph>)
Multi-step executable paths verified with closed-loop solve (3-SAT → SAT → MIS)
Reduction graph JSON export unchanged for existing reductions

🤖 Generated with Claude Code

codecov · 2026-02-15T15:39:19Z

Codecov Report

❌ Patch coverage is 94.66019% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.36%. Comparing base (148ef7f) to head (54206a8).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/rules/graph.rs	96.67%	11 Missing ⚠️
src/models/graph/kcoloring.rs	28.57%	10 Missing ⚠️
src/rules/mod.rs	33.33%	4 Missing ⚠️
src/rules/spinglass_casts.rs	0.00%	2 Missing ⚠️
src/unit_tests/rules/graph.rs	98.89%	2 Missing ⚠️
src/unit_tests/rules/registry.rs	77.77%	2 Missing ⚠️
src/rules/maximumsetpacking_casts.rs	0.00%	1 Missing ⚠️
src/unit_tests/reduction_graph.rs	93.75%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #72      +/-   ##
==========================================
- Coverage   96.77%   96.36%   -0.41%     
==========================================
  Files         189      191       +2     
  Lines       25906    25494     -412     
==========================================
- Hits        25070    24568     -502     
- Misses        836      926      +90

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

Replace auto-generated natural edges with explicit #[reduction] variant cast declarations. The reduction graph now uses variant-level nodes (each node is a unique problem_name + variant pair) with all edges coming from inventory registrations. Key changes: - Add impl_variant_reduction! macro: `Problem, <Src> => <Dst>, fields, closure` Single-arm design, works with any number of type parameters - Add ReductionOverhead::identity() for variant casts - Create 5 variant cast files (8 edges total) for KColoring, KSatisfiability, MaximumIndependentSet, MaximumSetPacking, SpinGlass - Refactor ReductionGraph from name-level to variant-level nodes - Remove old infrastructure: VariantTypeEntry inventory, build_variant_hierarchy, is_variant_reducible, EdgeKind::NaturalCast, resolve_path - Update docs: design.md, getting-started.md, introduction.md, reduction-graph.js - Interactive graph: dashed edges for variant casts (same name + identity overhead) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Refactors the variant/cast system to remove auto-generated “natural” edges and instead model all variant casts as explicit #[reduction] registrations, moving the runtime reduction graph from name-level nodes to variant-level (problem_name, variant) nodes.

Changes:

Remove VariantTypeEntry/inventory-driven variant hierarchy and natural-edge infrastructure; rely on explicit cast reductions.
Rework ReductionGraph to build a variant-level graph directly from ReductionEntry inventory, and update JSON export accordingly.
Add impl_variant_reduction! + ReductionOverhead::identity() and introduce explicit cast modules (MIS/KColoring/K-SAT/SetPacking/SpinGlass); update tests + docs/visualization.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/variant.rs	Removes runtime `VariantTypeEntry` registration; keeps trait-level variant metadata only.
src/unit_tests/variant.rs	Drops inventory-based registration assertions; keeps trait/cast behavior tests.
src/unit_tests/rules/natural.rs	Deletes tests for auto-inserted natural casts (feature removed).
src/unit_tests/rules/graph.rs	Updates tests for variant-level graph behavior, JSON expectations, and removes natural-edge checks.
src/unit_tests/reduction_graph.rs	Removes hierarchy/applicability tests tied to removed subtype infrastructure.
src/unit_tests/graph_types.rs	Removes inventory registration tests for graph/weight variants.
src/topology/triangular_subgraph.rs	Removes `VariantTypeEntry` inventory submission.
src/topology/kings_subgraph.rs	Removes `VariantTypeEntry` inventory submission.
src/rules/spinglass_casts.rs	Adds explicit variant-cast reduction for SpinGlass weight widening.
src/rules/registry.rs	Adds `ReductionOverhead::identity()` helper for size-preserving variant casts.
src/rules/natural.rs	Deletes natural-edge module (feature removed).
src/rules/mod.rs	Adds `impl_variant_reduction!` macro and wires new cast modules into rules module.
src/rules/maximumsetpacking_casts.rs	Adds explicit cast reduction for MaximumSetPacking weight widening.
src/rules/maximumindependentset_casts.rs	Adds explicit cast reductions for MIS graph relaxations (subtype-to-parent chain).
src/rules/ksatisfiability_casts.rs	Adds explicit cast reductions for `K2/K3 -> KN`.
src/rules/kcoloring_casts.rs	Adds explicit cast reduction for `K3 -> KN`.
src/rules/graph.rs	Major rewrite: variant-level nodes, Dijkstra over variant graph, updated JSON export, removes natural-edge logic.
src/graph_types.rs	Updates docs to reflect `VariantParam`-based hierarchy declaration.
docs/src/static/reduction-graph.js	Updates terminology/visual styling logic from “natural cast” to “variant cast”.
docs/src/reductions/reduction_graph.json	Regenerates graph JSON with doc_paths for cast edges and updated ordering.
docs/src/introduction.md	Updates legend label to “Variant Cast”.
docs/src/getting-started.md	Updates narrative and example to use explicit variant-cast reductions.
docs/src/design.md	Updates architecture/design narrative for explicit casts + variant-level graph.

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

Copilot · 2026-02-15T17:05:22Z

src/rules/graph.rs

+                let paths: Vec<Vec<NodeIndex>> = all_simple_paths::<
+                    Vec<NodeIndex>,
+                    _,
+                    std::hash::RandomState,
+                >(
+                    &self.graph, src_idx, dst_idx, 0, None


find_paths_by_name uses petgraph::algo::all_simple_paths with no maximum length, and now does this across all variant-node pairs. With variant-level graphs this can grow combinatorially and become very expensive (or hang) as more variants/reductions are added. For shortest-path use cases, prefer BFS/Dijkstra directly; for enumerating paths, consider adding a depth cap or exposing a parameter so callers can bound the search.

Suggested change

let paths: Vec<Vec<NodeIndex>> = all_simple_paths::<

Vec<NodeIndex>,

_,

std::hash::RandomState,

>(

&self.graph, src_idx, dst_idx, 0, None

// Bound the maximum path length by the number of nodes in the graph.

// For simple paths, this does not exclude any valid paths but makes the

// search space explicitly finite.

let max_len = self.graph.node_count();

let paths: Vec<Vec<NodeIndex>> = all_simple_paths::<

Vec<NodeIndex>,

_,

std::hash::RandomState,

>(

&self.graph, src_idx, dst_idx, 0, Some(max_len)

all_simple_paths already cannot exceed node_count() for simple paths by definition (no repeated nodes), so this cap is a no-op. The combinatorial concern is real but comes from the O(n²) loop over (src, dst) variant pairs, which the depth cap does not address. Leaving as-is.

Copilot · 2026-02-15T17:05:23Z

src/rules/graph.rs

+    /// Find the best matching `ReductionEntry` for a (source_name, target_name) pair
+    /// given the caller's current source variant.
+    ///
+    /// First tries an exact match on the source variant. If no exact match is found,
+    /// falls back to a name-only match (returning the first entry whose source and
+    /// target names match). This allows looking up overhead for specific variants
+    /// (e.g., `K3`) when only the general variant (e.g., `KN`) is registered.
+    pub fn find_best_entry(
+        &self,
+        source_name: &str,
+        target_name: &str,
+        current_variant: &BTreeMap<String, String>,
+    ) -> Option<MatchedEntry> {
+        let mut fallback: Option<MatchedEntry> = None;
+
+        for entry in inventory::iter::<ReductionEntry> {
+            if entry.source_name != source_name || entry.target_name != target_name {
+                continue;
+            }
+
+            let entry_source = Self::variant_to_map(&entry.source_variant());
+            let entry_target = Self::variant_to_map(&entry.target_variant());
+
+            // Exact match on source variant — return immediately
+            if current_variant == &entry_source {
+                return Some(MatchedEntry {
+                    source_variant: entry_source,
+                    target_variant: entry_target,
+                    overhead: entry.overhead(),
+                });
+            }
+
+            // Remember the first name-only match as a fallback
+            if fallback.is_none() {
+                fallback = Some(MatchedEntry {
+                    source_variant: entry_source,
+                    target_variant: entry_target,
+                    overhead: entry.overhead(),
+                });
+            }
+        }
+
+        fallback
+    }


find_best_entry falls back to the first name-only match when there is no exact source-variant match. With multiple variant-specific reductions between the same names (e.g., KSatisfiability<K2> and KSatisfiability<K3> -> QUBO), this can silently return the wrong overhead (and wrong target variant) depending on inventory iteration order. If variant-level edges are intended to be explicit, it would be safer to return None when no exact match exists; otherwise, the fallback should implement a deterministic and correct matching rule (e.g., subtype/compatibility) rather than “first seen.”

The fallback is intentional: specific variants (e.g. K3) may not have their own #[reduction] entry, but the general variant (KN) covers them with the same overhead polynomial. This is only used by the JSON export pipeline (export::lookup_overhead). Added a clarifying doc comment in 05390bb.

Copilot · 2026-02-15T17:05:23Z

src/rules/graph.rs

    pub fn find_cheapest_path<C: PathCostFn>(
        &self,
        source: (&str, &str),
        target: (&str, &str),
        input_size: &ProblemSize,
        cost_fn: &C,
    ) -> Option<ReductionPath> {
-        let src_idx = *self.name_indices.get(source.0)?;
-        let dst_idx = *self.name_indices.get(target.0)?;
+        // Find source nodes matching the name (we try all variant nodes for that name)
+        let src_nodes = self.name_to_nodes.get(source.0)?;
+        let dst_nodes = self.name_to_nodes.get(target.0)?;

-        let mut costs: HashMap<NodeIndex, f64> = HashMap::new();
-        let mut sizes: HashMap<NodeIndex, ProblemSize> = HashMap::new();
-        let mut prev: HashMap<NodeIndex, (NodeIndex, petgraph::graph::EdgeIndex)> = HashMap::new();
-        let mut heap = BinaryHeap::new();
+        // Build set of target node indices for quick lookup
+        let dst_set: HashSet<NodeIndex> = dst_nodes.iter().copied().collect();



find_cheapest_path still accepts (problem_name, graph_type) tuples but the graph_type values are no longer used to constrain the start/end variants. As written, the search tries all variants of source.0 and target.0, so callers can get a path that doesn't correspond to the requested graph type. Either remove the graph_type parameter from the public API or use it to select matching variant nodes (e.g., filter nodes by variant["graph"] == source.1/target.1).

Fixed in 05390bb. Added filter_by_graph_type() that constrains source/target nodes to those whose variant["graph"] matches the requested graph type. Nodes without a "graph" key (QUBO, ILP, etc.) pass through unconditionally.

Copilot · 2026-02-15T17:05:23Z

src/rules/graph.rs

        path.reverse();
-        ReductionPath { type_names: path }
+        // Deduplicate consecutive names (variant-level hops within same problem)
+        let mut deduped = Vec::new();
+        for name in &path {
+            if deduped.last() != Some(name) {
+                deduped.push(*name);
+            }
+        }
+        ReductionPath {
+            type_names: deduped,
+        }
    }


reconstruct_path_names deduplicates consecutive same-name nodes, which hides intra-problem variant-cast edges from the returned ReductionPath. This can make ReductionPath::len() disagree with what the cost function actually optimized over (e.g., MinimizeSteps counts cast edges, but they get dropped here). Consider returning a variant-level path type (with variants/edges), or ensuring cost/path length semantics explicitly ignore intra-problem edges end-to-end (both during Dijkstra and in the returned structure).

Already resolved in later commits. reconstruct_path now returns full variant-level ReductionStep data (name + variant map per step) without any deduplication. The ReductionPath.type_names() method provides the deduplicated name-level view when needed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ction, remove Clone from ReductionResult - Make ReductionPath variant-level with Vec<ReductionStep> instead of Vec<&'static str> - Add type_names() method for backward-compatible name-level path access - Remove ResolvedPath and EdgeKind (superseded by variant-level ReductionPath) - Add ExecutablePath<S,T> for typed, executable reduction paths via find_executable_path() - Add ChainedReduction<S,T> for composing multi-step reductions at runtime - Remove Clone bound from ReductionResult trait (no callers clone through trait) - Remove #[allow(dead_code)] from reduce_fn now that it's used Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add test_chained_reduction_with_variant_casts that exercises find_executable_path across variant boundaries. Tests two paths: - MIS<UnitDiskGraph> -> MIS<SimpleGraph> (variant cast) -> MVC (reduction) - KSat<K3> -> Satisfiability -> MIS (multi-step cross-problem reduction) Both paths verify end-to-end correctness by solving the target problem and extracting/validating the solution in the source domain. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.

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

Copilot · 2026-02-16T00:59:46Z

problemreductions-macros/src/lib.rs

+                    let src = src.downcast_ref::<#source_type>()
+                        .expect("DynReductionResult: source type mismatch");


The macro uses expect for downcasting, which will panic with a generic message on type mismatch. Consider adding more context to the panic message to help debug issues, such as including the expected and actual type names (if feasible with the proc-macro context).

Suggested change

let src = src.downcast_ref::<#source_type>()

.expect("DynReductionResult: source type mismatch");

let src = src.downcast_ref::<#source_type>().unwrap_or_else(|| {

panic!(

"DynReductionResult: source type mismatch: expected `{}`, got `{}`",

std::any::type_name::<#source_type>(),

std::any::type_name_of_val(src),

)

});

Fixed in bf017c3. Panic message now includes both expected and actual type names via type_name_of_val.

Copilot · 2026-02-16T00:59:46Z

src/rules/graph.rs

+            // Remember the first name-only match as a fallback
+            if fallback.is_none() {
+                fallback = Some(MatchedEntry {
+                    source_variant: entry_source,
+                    target_variant: entry_target,
+                    overhead: entry.overhead(),
+                });
+            }
+        }
+
+        fallback


The fallback logic returns "the first name-only match" when no exact variant match is found. However, inventory::iter order may not be deterministic across builds. If multiple entries exist with the same source and target names but different variants, the fallback choice could vary. Consider either documenting this non-determinism or collecting all candidates and selecting based on a deterministic criterion (e.g., lexicographically smallest variant).

Same concern as comment 2810271711. The fallback is intentional for the JSON export pipeline — specific variants (e.g. K3) may not have their own entry, but the general variant (KN) covers them. Inventory iteration order doesn't matter here because all same-name reductions share the same overhead polynomial. Added clarifying doc comment previously.

Rework find_cheapest_path to accept exact source and target variant maps, matching a single node on each end. This mirrors Julia's ProblemReductions.jl approach where path finding is always variant-aware. - Source: exact (name, variant) node — single Dijkstra start point - Target: exact (name, variant) node — single destination - Expose variant_to_map as pub(crate) for test ergonomics - Simplify Dijkstra: no outer loop over source nodes - Clarify find_best_entry fallback doc (intentional for export) - Fix test_find_direct_path to not depend on HashMap iteration order Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Shrink the interactive edge hit region by setting overlay-padding to 0 and edge width to 1px. Change the docs badge from green to traditional blue. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…jkstra - Extract shared `dijkstra(src, dst, ...)` taking NodeIndex directly - Add `lookup_node(name, variant)` as first-class node lookup - Add `make_executable(path)` to convert ReductionPath → ExecutablePath - Remove `find_executable_path` — callers now compose find + make_executable - Update docs and tests to use the new two-step pattern Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Add examples/chained_reduction_ksat_to_mis.rs (KSat<K3> → MIS via ILP) - Use {{#include}} in getting-started.md to keep doc in sync with example - Make variant_to_map public for external use - Fix latent bug: use find_cheapest_path for variant-exact matching (find_shortest_path is name-based and may cross variant boundaries) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

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

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

Copilot · 2026-02-16T02:29:05Z

src/rules/graph.rs

+    pub fn reduce(&self, source: &S) -> ChainedReduction<S, T> {
+        let mut steps: Vec<Box<dyn DynReductionResult>> = Vec::new();
+        let step = (self.edge_fns[0])(source as &dyn Any);
+        steps.push(step);
+        for edge_fn in &self.edge_fns[1..] {
+            let step = {
+                let prev_target = steps.last().unwrap().target_problem_any();
+                edge_fn(prev_target)
+            };
+            steps.push(step);


ExecutablePath::reduce unconditionally indexes self.edge_fns[0]. If make_executable is called with a ReductionPath that has <2 steps (0 edges)—e.g., a source==target path—this will panic at runtime. Consider either (a) making make_executable return None when path.len() == 0, or (b) handling the empty case in reduce (e.g., return an error/Option, or a dedicated identity reduction type).

Fixed. make_executable now returns None for paths with <2 steps (no edges to execute).

Copilot · 2026-02-16T02:29:05Z

src/rules/graph.rs

+                        path.push(prev_node);
+                        current = prev_node;
+                    } else {
+                        break;


In dijkstra, path reconstruction breaks if a prev entry is missing, but still returns the partially reconstructed path. This can yield a path that doesn't start at src (silent corruption) if the predecessor map is inconsistent. It would be safer to return None (or debug_assert!) when current != src and prev.get(&current) is None (except for the src == dst case).

Suggested change

break;

debug_assert!(

false,

"inconsistent predecessor map while reconstructing path in dijkstra"

);

return None;

Fixed. Changed break to return None — if the predecessor chain is incomplete, the path is invalid.

Copilot · 2026-02-16T02:29:06Z

src/rules/kcoloring_casts.rs

+impl_variant_reduction!(
+    KColoring,
+    <K3, SimpleGraph> => <KN, SimpleGraph>,
+    fields: [num_vertices, num_colors],
+    |src| KColoring::from_graph(src.graph().clone())
+);


This variant-cast reduction produces a KColoring<KN, SimpleGraph> instance. However, KColoring explicitly panics for KN when used as a problem instance (e.g., dims() / num_colors() call K::K.expect("KN cannot be used as problem instance") in src/models/graph/kcoloring.rs). With make_executable, any runtime path that traverses this cast and then tries to solve or further reduce the KN instance can panic. Either avoid casting to KN for execution (register concrete K reductions in the graph), or change KColoring<KN, _> to carry runtime k so it is executable.

Suggested change

impl_variant_reduction!(

KColoring,

<K3, SimpleGraph> => <KN, SimpleGraph>,

fields: [num_vertices, num_colors],

|src| KColoring::from_graph(src.graph().clone())

);

// NOTE:

// We intentionally do not register a variant-cast reduction from

// `KColoring<K3, SimpleGraph>` to `KColoring<KN, SimpleGraph>` here.

// `KColoring<KN, _>` panics when used as a problem instance

// (e.g., in `dims()` / `num_colors()`), so constructing it as part

// of the executable reduction graph would introduce a runtime panic.

// If a KN-based representation is needed in the future, it must be

// implemented in a way that carries a runtime `k` and is safe to

// execute.

We do not consider this issue right now, maybe file an issue.

Fixed. Added num_colors: usize field to KColoring so KN variant carries the runtime k value. The cast now uses from_graph_with_k(src.graph().clone(), src.num_colors()). Also updated ReductionKColoringToILP and ReductionKColoringToQUBO to store and use num_colors instead of K::K.expect(...).

- dijkstra: return None on inconsistent predecessor map instead of partial path - make_executable: return None for paths with <2 steps (no edges) - KColoring: add num_colors field so KN variant carries runtime k value - Update coloring_ilp and coloring_qubo to use stored num_colors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Prevents type parameter / field inconsistency by making from_graph_with_k a compile error for concrete K types (K3, K4, etc.). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove num_vertices() and num_edges() from KColoring, MaxCut, MaximumMatching, and TravelingSalesman. Callers now use graph().num_vertices() and graph().num_edges() via the Graph trait, matching the pattern established for MIS and other types in PR #71. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Fix find_cheapest_path signature: now takes separate name + variant BTreeMap instead of (name, graph_type) tuple - Fix executable path example: use find_cheapest_path with variant maps instead of find_shortest_path (which can pick wrong variants) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GiggleLiu requested a review from Copilot February 15, 2026 17:00

GiggleLiu force-pushed the variant-refactor-plan branch from ff60d62 to cefa199 Compare February 15, 2026 17:00

Copilot started reviewing on behalf of GiggleLiu February 15, 2026 17:01 View session

Copilot AI reviewed Feb 15, 2026

View reviewed changes

GiggleLiu and others added 5 commits February 16, 2026 01:37

feat: add DynReductionResult trait and reduce_fn to ReductionEntry

924602a

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

refactor: store reduce_fn on graph edges alongside overhead

9ca28aa

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

docs: update getting-started and design for path-based reduction API

053efb8

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GiggleLiu requested a review from Copilot February 16, 2026 00:53

Copilot started reviewing on behalf of GiggleLiu February 16, 2026 00:53 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

GiggleLiu and others added 3 commits February 16, 2026 09:24

docs: reduce edge click area in reduction graph and use blue docs badge

841af5e

Shrink the interactive edge hit region by setting overlay-padding to 0 and edge width to 1px. Change the docs badge from green to traditional blue. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GiggleLiu force-pushed the variant-refactor-plan branch from 05390bb to bf017c3 Compare February 16, 2026 02:06

fix: improve downcast panic message with expected and actual type names

2bdbeba

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GiggleLiu changed the title ~~Variant system refactor: explicit cast reductions~~ refactor: variant-level reduction graph with path-based API Feb 16, 2026

GiggleLiu requested a review from Copilot February 16, 2026 02:21

Copilot started reviewing on behalf of GiggleLiu February 16, 2026 02:21 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

GiggleLiu and others added 4 commits February 16, 2026 13:30

refactor: restrict from_graph_with_k to KColoring<KN> only

05ca331

Prevents type parameter / field inconsistency by making from_graph_with_k a compile error for concrete K types (K3, K4, etc.). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

GiggleLiu merged commit 529114f into main Feb 16, 2026
3 checks passed

GiggleLiu mentioned this pull request Feb 16, 2026

Reduce between problem and its variants #46

Closed

		let src = src.downcast_ref::<#source_type>()
		.expect("DynReductionResult: source type mismatch");

-                    let src = src.downcast_ref::<#source_type>()
-                        .expect("DynReductionResult: source type mismatch");
+                    let src = src.downcast_ref::<#source_type>().unwrap_or_else(|| {
+                        panic!(
+                            "DynReductionResult: source type mismatch: expected `{}`, got `{}`",
+                            std::any::type_name::<#source_type>(),
+                            std::any::type_name_of_val(src),
+                        )
+                    });

-                        break;
+                        debug_assert!(
+                            false,
+                            "inconsistent predecessor map while reconstructing path in dijkstra"
+                        );
+                        return None;

-impl_variant_reduction!(
-    KColoring,
-    <K3, SimpleGraph> => <KN, SimpleGraph>,
-    fields: [num_vertices, num_colors],
-    |src| KColoring::from_graph(src.graph().clone())
-);
+// NOTE:
+// We intentionally do not register a variant-cast reduction from
+// `KColoring<K3, SimpleGraph>` to `KColoring<KN, SimpleGraph>` here.
+// `KColoring<KN, _>` panics when used as a problem instance
+// (e.g., in `dims()` / `num_colors()`), so constructing it as part
+// of the executable reduction graph would introduce a runtime panic.
+// If a KN-based representation is needed in the future, it must be
+// implemented in a way that carries a runtime `k` and is safe to
+// execute.

Comments

Conversation

GiggleLiu commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

codecov bot commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

GiggleLiu commented Feb 15, 2026 •

edited

Loading

codecov bot commented Feb 15, 2026 •

edited

Loading