Address codex xhigh review: target.data coherence, tighter tests, pub(crate)

Addresses remaining items from codex xhigh review on #1060 that this PR
introduced (or whose scope this PR widened):

Must-fix (correctness hole introduced by claiming BundleReplay "validates"
bundles without fully doing so):
- `BundleReplay::prepare` now serializes the chain's replayed target and
  checks it byte-equals `bundle.target.data`. Previously a tampered bundle
  where `target.data` disagreed with what `reduce_along_path` actually
  produced would silently pass prepare(): callers solved/validated against
  the bundle's stated target but extracted through a different chain target.
  Now rejected with a "`target.data` does not match" error, consistently
  across `pred solve`, `pred extract`, and the MCP solve tool.

Tests:
- Tighten `test_extract_roundtrip_mis_to_qubo` to assert
  `intermediate.solution` echoes the input target config exactly, and
  that the source solution is a binary vector of the right length whose
  ones-count matches the declared source evaluation.
- New `test_extract_rejects_tampered_target_data` regression test covering
  the coherence check, asserting it fires on both `pred extract` and
  `pred solve` (verifying the shared gate).

Nit:
- Narrow `BundleReplay` field visibility from `pub` to `pub(crate)` —
  this helper is an internal CLI abstraction, not an external API.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: isPANN

problemreductions-cli/src/dispatch.rs

@@ -120,20 +120,25 @@ impl LoadedProblem {
 /// (`pred solve <bundle>`, `pred extract <bundle>`, MCP `solve_problem`)
 /// share this setup so validation and error text stay in sync.
 pub struct BundleReplay {
-    pub source: LoadedProblem,
-    pub source_name: String,
-    pub target: LoadedProblem,
-    pub target_name: String,
-    pub chain: problemreductions::rules::ReductionChain,
+    pub(crate) source: LoadedProblem,
+    pub(crate) source_name: String,
+    pub(crate) target: LoadedProblem,
+    pub(crate) target_name: String,
+    pub(crate) chain: problemreductions::rules::ReductionChain,
 }
 
 impl BundleReplay {
     /// Validate the bundle and replay the reduction chain.
     ///
-    /// Checks: `path` has at least two steps; `path[0]` matches `source`;
-    /// `path[-1]` matches `target`. Then loads both problems, reconstructs
-    /// the `ReductionPath`, and calls `reduce_along_path`. Returns an error
-    /// (not a panic) for malformed bundles or aggregate-only paths.
+    /// Checks:
+    /// - `path` has at least two steps
+    /// - `path[0]` matches `source` (name + variant)
+    /// - `path[-1]` matches `target` (name + variant)
+    /// - serializing the chain's replayed target equals `bundle.target.data`
+    ///   (tampered/stale bundles where `target.data` disagrees with what
+    ///   `reduce_along_path` actually produced are rejected)
+    ///
+    /// Returns an error (not a panic) for malformed bundles or aggregate-only paths.
     pub fn prepare(bundle: &ReductionBundle) -> Result<Self> {
         if bundle.path.len() < 2 {
             anyhow::bail!(
@@ -190,6 +195,20 @@ impl BundleReplay {
                 "Bundle requires a witness-capable reduction path; this bundle cannot map a target solution back to the source."
             ))?;
 
+        // Coherence check: `bundle.target.data` must equal what replaying
+        // `source` along `path` actually produces. Without this, a caller
+        // could solve/validate against the bundle's stated target but then
+        // extract through a completely different chain target.
+        let replayed_target_data =
+            serialize_any_problem(&last.name, &last.variant, chain.target_problem_any())?;
+        if replayed_target_data != bundle.target.data {
+            anyhow::bail!(
+                "Malformed bundle: `target.data` does not match the result of replaying \
+                 `source` along `path`. The bundle is tampered or was produced by \
+                 incompatible code."
+            );
+        }
+
         Ok(Self {
             source,
             source_name,

problemreductions-cli/tests/cli_tests.rs

@@ -8830,8 +8830,34 @@ fn test_extract_roundtrip_mis_to_qubo() {
     // extract on pred-solve's own target config must round-trip to the same source evaluation.
     assert_eq!(json["evaluation"].as_str().unwrap(), expected_source_eval);
     assert_eq!(json["intermediate"]["problem"].as_str().unwrap(), "QUBO");
-    assert!(json["solution"].is_array());
-    assert!(json["intermediate"]["solution"].is_array());
+
+    // intermediate.solution must be exactly the target config we passed in
+    // (extract echoes the input target config unchanged).
+    let expected_target: Vec<serde_json::Value> = target_cfg
+        .split(',')
+        .map(|s| serde_json::json!(s.parse::<u64>().unwrap()))
+        .collect();
+    assert_eq!(
+        json["intermediate"]["solution"].as_array().unwrap(),
+        &expected_target
+    );
+
+    // Source config is over 4 MIS variables and must describe an independent set
+    // whose size matches `expected_source_eval` (e.g. "Max(2)" -> 2 ones).
+    let source_sol: Vec<u64> = json["solution"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .map(|v| v.as_u64().unwrap())
+        .collect();
+    assert_eq!(source_sol.len(), 4);
+    assert!(source_sol.iter().all(|b| *b == 0 || *b == 1));
+    let ones = source_sol.iter().filter(|b| **b == 1).count();
+    assert_eq!(
+        expected_source_eval,
+        format!("Max({ones})"),
+        "MIS size in solution should match declared evaluation"
+    );
 
     std::fs::remove_file(&problem_file).ok();
     std::fs::remove_file(&bundle_file).ok();
@@ -9034,6 +9060,92 @@ fn test_extract_rejects_malformed_bundle_path_source_mismatch() {
     std::fs::remove_file(&tampered_file).ok();
 }
 
+#[test]
+fn test_extract_rejects_tampered_target_data() {
+    use std::io::Write;
+
+    let problem_file = std::env::temp_dir().join("pred_test_extract_tampered_target_in.json");
+    let bundle_file = std::env::temp_dir().join("pred_test_extract_tampered_target_bundle.json");
+    let tampered_file =
+        std::env::temp_dir().join("pred_test_extract_tampered_target_tampered.json");
+
+    pred()
+        .args([
+            "-o",
+            problem_file.to_str().unwrap(),
+            "create",
+            "MIS",
+            "--graph",
+            "0-1,1-2,2-3",
+        ])
+        .output()
+        .unwrap();
+    pred()
+        .args([
+            "-o",
+            bundle_file.to_str().unwrap(),
+            "reduce",
+            problem_file.to_str().unwrap(),
+            "--to",
+            "QUBO",
+        ])
+        .output()
+        .unwrap();
+
+    // Tamper: flip one QUBO matrix entry so target.data no longer matches
+    // what the reduction chain actually produces.
+    let bundle_text = std::fs::read_to_string(&bundle_file).unwrap();
+    let mut bundle: serde_json::Value = serde_json::from_str(&bundle_text).unwrap();
+    bundle["target"]["data"]["matrix"][0][0] = serde_json::json!(999.0);
+    let mut f = std::fs::File::create(&tampered_file).unwrap();
+    f.write_all(bundle.to_string().as_bytes()).unwrap();
+
+    // Any config long enough to reach the coherence check; it must fail before
+    // config validation kicks in because prepare() runs first.
+    let (target_cfg, _) = extract_test_solve_bundle(&bundle_file);
+    let extract_out = pred()
+        .args([
+            "extract",
+            tampered_file.to_str().unwrap(),
+            "--config",
+            &target_cfg,
+        ])
+        .output()
+        .unwrap();
+    assert!(
+        !extract_out.status.success(),
+        "expected failure on tampered target.data; stdout: {}",
+        String::from_utf8_lossy(&extract_out.stdout)
+    );
+    let stderr = String::from_utf8(extract_out.stderr).unwrap();
+    assert!(
+        stderr.contains("`target.data` does not match"),
+        "unexpected stderr: {stderr}"
+    );
+
+    // Same check must also fire through `pred solve` on the tampered bundle —
+    // BundleReplay::prepare is the shared gate.
+    let solve_out = pred()
+        .args([
+            "solve",
+            tampered_file.to_str().unwrap(),
+            "--solver",
+            "brute-force",
+        ])
+        .output()
+        .unwrap();
+    assert!(!solve_out.status.success());
+    let solve_err = String::from_utf8(solve_out.stderr).unwrap();
+    assert!(
+        solve_err.contains("`target.data` does not match"),
+        "pred solve should also reject tampered bundles; got: {solve_err}"
+    );
+
+    std::fs::remove_file(&problem_file).ok();
+    std::fs::remove_file(&bundle_file).ok();
+    std::fs::remove_file(&tampered_file).ok();
+}
+
 #[test]
 fn test_extract_reads_bundle_from_stdin() {
     use std::io::Write;