Skip to content

Commit fbaed09

Browse files
isPANNclaude
andcommitted
Factor bundle replay into BundleReplay helper; unify solve/extract/MCP
Codex review item 4. Before: `solve_bundle` (CLI), `solve_bundle_inner` (MCP), and `extract` each had their own copy of the "load bundle, validate, reconstruct ReductionPath, call reduce_along_path" flow — three places to drift out of sync, and only `extract` had the endpoint-vs-source/target validation added in the previous commit. Now `BundleReplay::prepare` is the single entry point: validates bundle.path length and endpoint consistency with source/target, loads both problems, rebuilds the path, and replays to a `ReductionChain`. Callers just pick their own way to produce a target config (solver vs external input) and call `replay.extract(target_config)`. Benefit: the malformed-bundle check now protects `pred solve` and the MCP bundle-solve tool too, not just `pred extract`. Also: tests derive the target config from `pred solve --solver brute-force` instead of hardcoding it, so they pass under both default and `--features mcp` builds (which pick different reduction paths, MIS->...->ILP->QUBO vs MIS->...->MaxSetPacking->QUBO). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5a028dc commit fbaed09

5 files changed

Lines changed: 191 additions & 194 deletions

File tree

Lines changed: 10 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
use crate::dispatch::{load_problem, read_input, ReductionBundle};
1+
use crate::dispatch::{read_input, BundleReplay, ReductionBundle};
22
use crate::output::OutputConfig;
33
use anyhow::{Context, Result};
4-
use problemreductions::rules::{ReductionGraph, ReductionPath, ReductionStep};
54
use std::path::Path;
65

76
/// Extract a source-space configuration from a target-space configuration and a reduction bundle.
@@ -40,42 +39,14 @@ pub fn extract(input: &Path, config_str: &str, out: &OutputConfig) -> Result<()>
4039
.collect::<Result<Vec<_>>>()?
4140
};
4241

43-
// Validate bundle self-consistency before trusting it.
44-
if bundle.path.len() < 2 {
45-
anyhow::bail!(
46-
"Malformed bundle: `path` must contain at least two steps (source and target), got {}",
47-
bundle.path.len()
48-
);
49-
}
50-
let first = bundle.path.first().unwrap();
51-
let last = bundle.path.last().unwrap();
52-
if first.name != bundle.source.problem_type || first.variant != bundle.source.variant {
53-
anyhow::bail!(
54-
"Malformed bundle: path starts with {} but source is {}",
55-
format_step(&first.name, &first.variant),
56-
format_step(&bundle.source.problem_type, &bundle.source.variant),
57-
);
58-
}
59-
if last.name != bundle.target.problem_type || last.variant != bundle.target.variant {
60-
anyhow::bail!(
61-
"Malformed bundle: path ends with {} but target is {}",
62-
format_step(&last.name, &last.variant),
63-
format_step(&bundle.target.problem_type, &bundle.target.variant),
64-
);
65-
}
42+
let replay = BundleReplay::prepare(&bundle)?;
6643

67-
let target = load_problem(
68-
&bundle.target.problem_type,
69-
&bundle.target.variant,
70-
bundle.target.data.clone(),
71-
)?;
72-
let target_name = target.problem_name().to_string();
73-
let target_dims = target.dims_dyn();
44+
let target_dims = replay.target.dims_dyn();
7445
if target_config.len() != target_dims.len() {
7546
anyhow::bail!(
7647
"Target config has {} values but target problem {} has {} variables",
7748
target_config.len(),
78-
target_name,
49+
replay.target_name,
7950
target_dims.len()
8051
);
8152
}
@@ -87,71 +58,30 @@ pub fn extract(input: &Path, config_str: &str, out: &OutputConfig) -> Result<()>
8758
);
8859
}
8960
}
90-
let target_eval = target.evaluate_dyn(&target_config);
61+
let target_eval = replay.target.evaluate_dyn(&target_config);
9162

92-
let source = load_problem(
93-
&bundle.source.problem_type,
94-
&bundle.source.variant,
95-
bundle.source.data.clone(),
96-
)?;
97-
let source_name = source.problem_name().to_string();
98-
99-
let graph = ReductionGraph::new();
100-
let reduction_path = ReductionPath {
101-
steps: bundle
102-
.path
103-
.iter()
104-
.map(|s| ReductionStep {
105-
name: s.name.clone(),
106-
variant: s.variant.clone(),
107-
})
108-
.collect(),
109-
};
110-
111-
let chain = graph
112-
.reduce_along_path(&reduction_path, source.as_any())
113-
.ok_or_else(|| {
114-
anyhow::anyhow!(
115-
"Bundle extraction requires a witness-capable reduction path; \
116-
this bundle's path cannot map a target solution back to the source."
117-
)
118-
})?;
119-
120-
let source_config = chain.extract_solution(&target_config);
121-
let source_eval = source.evaluate_dyn(&source_config);
63+
let (source_config, source_eval) = replay.extract(&target_config);
12264

12365
let text = format!(
12466
"Problem: {}\nSolver: external (via {})\nSolution: {:?}\nEvaluation: {}",
125-
source_name, target_name, source_config, source_eval,
67+
replay.source_name, replay.target_name, source_config, source_eval,
12668
);
12769

12870
// Schema aligned with `pred solve` on a bundle: `problem`, `reduced_to`, `solution`,
12971
// `evaluation`, `intermediate { problem, solution, evaluation }`. `solver` is "external"
13072
// to signal that pred did not run a solver — the target config came from outside.
13173
let json = serde_json::json!({
132-
"problem": source_name,
74+
"problem": replay.source_name,
13375
"solver": "external",
134-
"reduced_to": target_name,
76+
"reduced_to": replay.target_name,
13577
"solution": source_config,
13678
"evaluation": source_eval,
13779
"intermediate": {
138-
"problem": target_name,
80+
"problem": replay.target_name,
13981
"solution": target_config,
14082
"evaluation": target_eval,
14183
},
14284
});
14385

14486
out.emit_with_default_name("pred_extract.json", &text, &json)
14587
}
146-
147-
fn format_step(name: &str, variant: &std::collections::BTreeMap<String, String>) -> String {
148-
if variant.is_empty() {
149-
name.to_string()
150-
} else {
151-
let parts: Vec<String> = variant
152-
.iter()
153-
.map(|(k, v)| format!("{}={}", k, v))
154-
.collect();
155-
format!("{}{{{}}}", name, parts.join(", "))
156-
}
157-
}

problemreductions-cli/src/commands/solve.rs

Lines changed: 13 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
use crate::dispatch::{load_problem, read_input, ProblemJson, ReductionBundle};
1+
use crate::dispatch::{load_problem, read_input, BundleReplay, ProblemJson, ReductionBundle};
22
use crate::output::OutputConfig;
33
use anyhow::{Context, Result};
4-
use problemreductions::rules::ReductionGraph;
54
use std::path::Path;
65
use std::time::Duration;
76

@@ -166,75 +165,39 @@ fn solve_problem(
166165

167166
/// Solve a reduction bundle: solve the target problem, then map the solution back.
168167
fn solve_bundle(bundle: ReductionBundle, solver_name: &str, out: &OutputConfig) -> Result<()> {
169-
// 1. Load the target problem from the bundle
170-
let target = load_problem(
171-
&bundle.target.problem_type,
172-
&bundle.target.variant,
173-
bundle.target.data.clone(),
174-
)?;
175-
let target_name = target.problem_name();
168+
let replay = BundleReplay::prepare(&bundle)?;
176169

177-
// 2. Solve the target problem
178170
let target_result = match solver_name {
179-
"brute-force" => target.solve_brute_force_witness().ok_or_else(|| {
171+
"brute-force" => replay.target.solve_brute_force_witness().ok_or_else(|| {
180172
anyhow::anyhow!(
181173
"Bundle solving requires a witness-capable target problem and witness-capable reduction path; {} only supports aggregate-value solving.",
182-
target_name
174+
replay.target_name
183175
)
184176
})?,
185-
"ilp" => target.solve_with_ilp().map_err(add_ilp_solver_hint)?,
186-
"customized" => target
177+
"ilp" => replay.target.solve_with_ilp().map_err(add_ilp_solver_hint)?,
178+
"customized" => replay
179+
.target
187180
.solve_with_customized()
188181
.map_err(add_customized_solver_hint)?,
189182
_ => unreachable!(),
190183
};
191184

192-
// 3. Load source problem and re-execute the reduction chain to get extract_solution
193-
let source = load_problem(
194-
&bundle.source.problem_type,
195-
&bundle.source.variant,
196-
bundle.source.data.clone(),
197-
)?;
198-
let source_name = source.problem_name();
185+
let (source_config, source_eval) = replay.extract(&target_result.config);
199186

200-
let graph = ReductionGraph::new();
201-
202-
// Reconstruct the ReductionPath from the bundle's path steps
203-
let reduction_path = problemreductions::rules::ReductionPath {
204-
steps: bundle
205-
.path
206-
.iter()
207-
.map(|s| problemreductions::rules::ReductionStep {
208-
name: s.name.clone(),
209-
variant: s.variant.clone(),
210-
})
211-
.collect(),
212-
};
213-
214-
let chain = graph
215-
.reduce_along_path(&reduction_path, source.as_any())
216-
.ok_or_else(|| anyhow::anyhow!(
217-
"Bundle solving requires a witness-capable reduction path; this bundle cannot recover a source solution."
218-
))?;
219-
220-
// 4. Extract solution back to source problem space
221-
let source_config = chain.extract_solution(&target_result.config);
222-
let source_eval = source.evaluate_dyn(&source_config);
223-
224-
let solver_desc = format!("{} (via {})", solver_name, target_name);
187+
let solver_desc = format!("{} (via {})", solver_name, replay.target_name);
225188
let text = format!(
226189
"Problem: {}\nSolver: {}\nSolution: {:?}\nEvaluation: {}",
227-
source_name, solver_desc, source_config, source_eval,
190+
replay.source_name, solver_desc, source_config, source_eval,
228191
);
229192

230193
let json = serde_json::json!({
231-
"problem": source_name,
194+
"problem": replay.source_name,
232195
"solver": solver_name,
233-
"reduced_to": target_name,
196+
"reduced_to": replay.target_name,
234197
"solution": source_config,
235198
"evaluation": source_eval,
236199
"intermediate": {
237-
"problem": target_name,
200+
"problem": replay.target_name,
238201
"solution": target_result.config,
239202
"evaluation": target_result.evaluation,
240203
},

problemreductions-cli/src/dispatch.rs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,111 @@ impl LoadedProblem {
114114
}
115115
}
116116

117+
/// A validated reduction bundle ready to replay:
118+
/// source, target, and the reconstructed reduction chain. Construct via
119+
/// [`BundleReplay::prepare`]. All three CLI/MCP bundle workflows
120+
/// (`pred solve <bundle>`, `pred extract <bundle>`, MCP `solve_problem`)
121+
/// share this setup so validation and error text stay in sync.
122+
pub struct BundleReplay {
123+
pub source: LoadedProblem,
124+
pub source_name: String,
125+
pub target: LoadedProblem,
126+
pub target_name: String,
127+
pub chain: problemreductions::rules::ReductionChain,
128+
}
129+
130+
impl BundleReplay {
131+
/// Validate the bundle and replay the reduction chain.
132+
///
133+
/// Checks: `path` has at least two steps; `path[0]` matches `source`;
134+
/// `path[-1]` matches `target`. Then loads both problems, reconstructs
135+
/// the `ReductionPath`, and calls `reduce_along_path`. Returns an error
136+
/// (not a panic) for malformed bundles or aggregate-only paths.
137+
pub fn prepare(bundle: &ReductionBundle) -> Result<Self> {
138+
if bundle.path.len() < 2 {
139+
anyhow::bail!(
140+
"Malformed bundle: `path` must contain at least two steps (source and target), got {}",
141+
bundle.path.len()
142+
);
143+
}
144+
let first = bundle.path.first().unwrap();
145+
let last = bundle.path.last().unwrap();
146+
if first.name != bundle.source.problem_type || first.variant != bundle.source.variant {
147+
anyhow::bail!(
148+
"Malformed bundle: path starts with {} but source is {}",
149+
format_step(&first.name, &first.variant),
150+
format_step(&bundle.source.problem_type, &bundle.source.variant),
151+
);
152+
}
153+
if last.name != bundle.target.problem_type || last.variant != bundle.target.variant {
154+
anyhow::bail!(
155+
"Malformed bundle: path ends with {} but target is {}",
156+
format_step(&last.name, &last.variant),
157+
format_step(&bundle.target.problem_type, &bundle.target.variant),
158+
);
159+
}
160+
161+
let source = load_problem(
162+
&bundle.source.problem_type,
163+
&bundle.source.variant,
164+
bundle.source.data.clone(),
165+
)?;
166+
let source_name = source.problem_name().to_string();
167+
168+
let target = load_problem(
169+
&bundle.target.problem_type,
170+
&bundle.target.variant,
171+
bundle.target.data.clone(),
172+
)?;
173+
let target_name = target.problem_name().to_string();
174+
175+
let reduction_path = problemreductions::rules::ReductionPath {
176+
steps: bundle
177+
.path
178+
.iter()
179+
.map(|s| problemreductions::rules::ReductionStep {
180+
name: s.name.clone(),
181+
variant: s.variant.clone(),
182+
})
183+
.collect(),
184+
};
185+
186+
let graph = ReductionGraph::new();
187+
let chain = graph
188+
.reduce_along_path(&reduction_path, source.as_any())
189+
.ok_or_else(|| anyhow::anyhow!(
190+
"Bundle requires a witness-capable reduction path; this bundle cannot map a target solution back to the source."
191+
))?;
192+
193+
Ok(Self {
194+
source,
195+
source_name,
196+
target,
197+
target_name,
198+
chain,
199+
})
200+
}
201+
202+
/// Map a target-space configuration back to the source space and evaluate it.
203+
pub fn extract(&self, target_config: &[usize]) -> (Vec<usize>, String) {
204+
let source_config = self.chain.extract_solution(target_config);
205+
let source_eval = self.source.evaluate_dyn(&source_config);
206+
(source_config, source_eval)
207+
}
208+
}
209+
210+
fn format_step(name: &str, variant: &BTreeMap<String, String>) -> String {
211+
if variant.is_empty() {
212+
name.to_string()
213+
} else {
214+
let parts: Vec<String> = variant
215+
.iter()
216+
.map(|(k, v)| format!("{}={}", k, v))
217+
.collect();
218+
format!("{}{{{}}}", name, parts.join(", "))
219+
}
220+
}
221+
117222
/// Load a problem from JSON type/variant/data.
118223
pub fn load_problem(
119224
name: &str,

0 commit comments

Comments
 (0)