Skip to content

Commit 6136ea8

Browse files
GiggleLiuclaude
andauthored
Align CBM serialized form with declared schema (#1065)
* Align CBM serialized form with declared schema `ConsecutiveBlockMinimization`'s `ProblemSchemaEntry::fields` declares `matrix` and `bound`, but its serialized form also wrote `num_rows` and `num_cols`, and its `try_from` rejected JSON that lacked them. This made the schema-driven `pred create` path fail with "missing field `num_rows`" for any caller that built JSON from the declared schema fields. `num_rows` and `num_cols` are fully derived from `matrix` (just `matrix.len()` and `matrix[0].len()`), so they don't belong in the wire format. Switch to symmetric `serde(try_from / into = Def)` so both serialize and deserialize round-trip through the minimal `{matrix, bound}` form, and let `try_new` recompute the cached dimensions on the way in. The cached fields stay on the in-memory struct for fast getter access. Replace the now-unreachable inconsistent-dimensions test with one that pins the minimal wire format and one that exercises ragged-matrix rejection — the actual remaining input validation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: update CBM CLI rejection test for new wire format The CI Test job for #1065 failed because cli_tests.rs:445 still asserted the old "num_cols must match matrix column count" error. Under the new serde definition, extra num_rows/num_cols fields in the JSON are silently ignored (serde's default behavior for unknown fields when not using deny_unknown_fields), so the previous JSON now deserializes successfully and the CLI no longer rejects it. Switch the test to exercise the actual remaining input validation — ragged-matrix rejection — which is what `validate_matrix_dimensions` guards against and what the in-tree unit test now also covers. Rename the test accordingly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent e3b8870 commit 6136ea8

3 files changed

Lines changed: 31 additions & 26 deletions

File tree

problemreductions-cli/tests/cli_tests.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -423,17 +423,15 @@ fn test_evaluate_sat() {
423423
}
424424

425425
#[test]
426-
fn test_evaluate_consecutive_block_minimization_rejects_inconsistent_dimensions() {
426+
fn test_evaluate_consecutive_block_minimization_rejects_ragged_matrix() {
427427
let problem_json = r#"{
428428
"type": "ConsecutiveBlockMinimization",
429429
"data": {
430-
"matrix": [[true]],
431-
"num_rows": 1,
432-
"num_cols": 2,
430+
"matrix": [[true, false], [true]],
433431
"bound": 1
434432
}
435433
}"#;
436-
let tmp = std::env::temp_dir().join("pred_test_eval_cbm_invalid_dims.json");
434+
let tmp = std::env::temp_dir().join("pred_test_eval_cbm_ragged_matrix.json");
437435
std::fs::write(&tmp, problem_json).unwrap();
438436

439437
let output = pred()
@@ -442,7 +440,7 @@ fn test_evaluate_consecutive_block_minimization_rejects_inconsistent_dimensions(
442440
.unwrap();
443441
assert!(!output.status.success());
444442
let stderr = String::from_utf8_lossy(&output.stderr);
445-
assert!(stderr.contains("num_cols must match matrix column count"));
443+
assert!(stderr.contains("same length"));
446444
assert!(!stderr.contains("panicked at"), "stderr: {stderr}");
447445
std::fs::remove_file(&tmp).ok();
448446
}

src/models/algebraic/consecutive_block_minimization.rs

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ inventory::submit! {
5858
/// }
5959
/// ```
6060
#[derive(Debug, Clone, Serialize, Deserialize)]
61-
#[serde(try_from = "ConsecutiveBlockMinimizationDef")]
61+
#[serde(
62+
try_from = "ConsecutiveBlockMinimizationDef",
63+
into = "ConsecutiveBlockMinimizationDef"
64+
)]
6265
pub struct ConsecutiveBlockMinimization {
6366
/// The binary matrix A (m x n).
6467
matrix: Vec<Vec<bool>>,
@@ -184,32 +187,26 @@ crate::declare_variants! {
184187
default ConsecutiveBlockMinimization => "factorial(num_cols) * num_rows * num_cols",
185188
}
186189

187-
#[derive(Debug, Clone, Deserialize)]
190+
#[derive(Debug, Clone, Serialize, Deserialize)]
188191
struct ConsecutiveBlockMinimizationDef {
189192
matrix: Vec<Vec<bool>>,
190-
num_rows: usize,
191-
num_cols: usize,
192193
bound: i64,
193194
}
194195

195196
impl TryFrom<ConsecutiveBlockMinimizationDef> for ConsecutiveBlockMinimization {
196197
type Error = String;
197198

198199
fn try_from(value: ConsecutiveBlockMinimizationDef) -> Result<Self, Self::Error> {
199-
let problem = Self::try_new(value.matrix, value.bound)?;
200-
if value.num_rows != problem.num_rows {
201-
return Err(format!(
202-
"num_rows must match matrix row count ({})",
203-
problem.num_rows
204-
));
205-
}
206-
if value.num_cols != problem.num_cols {
207-
return Err(format!(
208-
"num_cols must match matrix column count ({})",
209-
problem.num_cols
210-
));
200+
Self::try_new(value.matrix, value.bound)
201+
}
202+
}
203+
204+
impl From<ConsecutiveBlockMinimization> for ConsecutiveBlockMinimizationDef {
205+
fn from(value: ConsecutiveBlockMinimization) -> Self {
206+
Self {
207+
matrix: value.matrix,
208+
bound: value.bound,
211209
}
212-
Ok(problem)
213210
}
214211
}
215212

src/unit_tests/models/algebraic/consecutive_block_minimization.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,20 @@ fn test_consecutive_block_minimization_serialization() {
9191
}
9292

9393
#[test]
94-
fn test_consecutive_block_minimization_deserialization_rejects_inconsistent_dimensions() {
95-
let json = r#"{"matrix":[[true]],"num_rows":1,"num_cols":2,"bound":1}"#;
94+
fn test_consecutive_block_minimization_serialization_omits_derived_fields() {
95+
let problem = ConsecutiveBlockMinimization::new(vec![vec![true, false], vec![false, true]], 2);
96+
let value: serde_json::Value = serde_json::to_value(&problem).unwrap();
97+
let obj = value.as_object().unwrap();
98+
assert_eq!(obj.len(), 2);
99+
assert!(obj.contains_key("matrix"));
100+
assert!(obj.contains_key("bound"));
101+
}
102+
103+
#[test]
104+
fn test_consecutive_block_minimization_deserialization_rejects_ragged_matrix() {
105+
let json = r#"{"matrix":[[true,false],[true]],"bound":1}"#;
96106
let err = serde_json::from_str::<ConsecutiveBlockMinimization>(json).unwrap_err();
97-
assert!(err.to_string().contains("num_cols"));
107+
assert!(err.to_string().contains("same length"));
98108
}
99109

100110
#[test]

0 commit comments

Comments
 (0)