feat: spreadsheet v2 (#13347)

codex-rs/artifact-spreadsheet/src/manager.rs

@@ -1,3 +1,4 @@
+use std::collections::BTreeMap;
 use std::collections::HashMap;
 use std::path::Path;
 use std::path::PathBuf;
@@ -102,6 +103,12 @@ impl SpreadsheetArtifactManager {
             "delete_sheet" => self.delete_sheet(request),
             "set_sheet_properties" => self.set_sheet_properties(request),
             "set_column_widths" => self.set_column_widths(request),
+            "set_column_widths_bulk" => self.set_column_widths_bulk(request),
+            "set_row_height" => self.set_row_height(request),
+            "set_row_heights" => self.set_row_heights(request),
+            "set_row_heights_bulk" => self.set_row_heights_bulk(request),
+            "get_row_height" => self.get_row_height(request),
+            "cleanup_and_validate_sheet" => self.cleanup_and_validate_sheet(request),
             "get_cell" => self.get_cell(request),
             "get_cell_by_indices" => self.get_cell_by_indices(request),
             "get_cell_field" => self.get_cell_field(request),
@@ -484,6 +491,180 @@ impl SpreadsheetArtifactManager {
         Ok(response)
     }
 
+    fn set_column_widths_bulk(
+        &mut self,
+        request: SpreadsheetArtifactRequest,
+    ) -> Result<SpreadsheetArtifactResponse, SpreadsheetArtifactError> {
+        let args: SetColumnWidthsBulkArgs = parse_args(&request.action, &request.args)?;
+        let width_count = args.widths.len();
+        let artifact_id = required_artifact_id(&request)?;
+        let artifact = self.get_artifact_mut(&artifact_id, &request.action)?;
+        let sheet_summary = {
+            let sheet = artifact.sheet_lookup_mut(
+                &request.action,
+                args.sheet_name.as_deref(),
+                args.sheet_index.map(|value| value as usize),
+            )?;
+            sheet.set_column_widths_bulk(&args.widths)?;
+            sheet.summary()
+        };
+        let mut response = SpreadsheetArtifactResponse::new(
+            artifact_id,
+            request.action,
+            format!(
+                "Updated {width_count} column width references on `{}`",
+                sheet_summary.name
+            ),
+            snapshot_for_artifact(artifact),
+        );
+        response.sheet_list = Some(vec![sheet_summary]);
+        Ok(response)
+    }
+
+    fn set_row_height(
+        &mut self,
+        request: SpreadsheetArtifactRequest,
+    ) -> Result<SpreadsheetArtifactResponse, SpreadsheetArtifactError> {
+        let args: SetRowHeightArgs = parse_args(&request.action, &request.args)?;
+        let artifact_id = required_artifact_id(&request)?;
+        let artifact = self.get_artifact_mut(&artifact_id, &request.action)?;
+        let (sheet_summary, row_height) = {
+            let sheet = artifact.sheet_lookup_mut(
+                &request.action,
+                args.sheet_name.as_deref(),
+                args.sheet_index.map(|value| value as usize),
+            )?;
+            sheet.set_row_height(args.row_index, args.height)?;
+            (sheet.summary(), sheet.get_row_height(args.row_index))
+        };
+        let mut response = SpreadsheetArtifactResponse::new(
+            artifact_id,
+            request.action,
+            format!(
+                "Updated row height for row {} on `{}`",
+                args.row_index, sheet_summary.name
+            ),
+            snapshot_for_artifact(artifact),
+        );
+        response.sheet_list = Some(vec![sheet_summary]);
+        response.row_height = row_height;
+        Ok(response)
+    }
+
+    fn set_row_heights(
+        &mut self,
+        request: SpreadsheetArtifactRequest,
+    ) -> Result<SpreadsheetArtifactResponse, SpreadsheetArtifactError> {
+        let args: SetRowHeightsArgs = parse_args(&request.action, &request.args)?;
+        let artifact_id = required_artifact_id(&request)?;
+        let artifact = self.get_artifact_mut(&artifact_id, &request.action)?;
+        let start = args.start_row_index.min(args.end_row_index);
+        let end = args.start_row_index.max(args.end_row_index);
+        let sheet_summary = {
+            let sheet = artifact.sheet_lookup_mut(
+                &request.action,
+                args.sheet_name.as_deref(),
+                args.sheet_index.map(|value| value as usize),
+            )?;
+            sheet.set_row_heights(args.start_row_index, args.end_row_index, args.height)?;
+            sheet.summary()
+        };
+        let mut response = SpreadsheetArtifactResponse::new(
+            artifact_id,
+            request.action,
+            format!(
+                "Updated row heights {start}:{end} on `{}`",
+                sheet_summary.name
+            ),
+            snapshot_for_artifact(artifact),
+        );
+        response.sheet_list = Some(vec![sheet_summary]);
+        Ok(response)
+    }
+
+    fn set_row_heights_bulk(
+        &mut self,
+        request: SpreadsheetArtifactRequest,
+    ) -> Result<SpreadsheetArtifactResponse, SpreadsheetArtifactError> {
+        let args: SetRowHeightsBulkArgs = parse_args(&request.action, &request.args)?;
+        let height_count = args.heights.len();
+        let artifact_id = required_artifact_id(&request)?;
+        let artifact = self.get_artifact_mut(&artifact_id, &request.action)?;
+        let sheet_summary = {
+            let sheet = artifact.sheet_lookup_mut(
+                &request.action,
+                args.sheet_name.as_deref(),
+                args.sheet_index.map(|value| value as usize),
+            )?;
+            sheet.set_row_heights_bulk(&args.heights)?;
+            sheet.summary()
+        };
+        let mut response = SpreadsheetArtifactResponse::new(
+            artifact_id,
+            request.action,
+            format!(
+                "Updated {height_count} row height entries on `{}`",
+                sheet_summary.name
+            ),
+            snapshot_for_artifact(artifact),
+        );
+        response.sheet_list = Some(vec![sheet_summary]);
+        Ok(response)
+    }
+
+    fn get_row_height(
+        &mut self,
+        request: SpreadsheetArtifactRequest,
+    ) -> Result<SpreadsheetArtifactResponse, SpreadsheetArtifactError> {
+        let args: GetRowHeightArgs = parse_args(&request.action, &request.args)?;
+        let artifact_id = required_artifact_id(&request)?;
+        let artifact = self.get_artifact(&artifact_id, &request.action)?;
+        let sheet = artifact.sheet_lookup(
+            &request.action,
+            args.sheet_name.as_deref(),
+            args.sheet_index.map(|value| value as usize),
+        )?;
+        let mut response = SpreadsheetArtifactResponse::new(
+            artifact_id,
+            request.action,
+            format!(
+                "Retrieved row height for row {} from `{}`",
+                args.row_index, sheet.name
+            ),
+            snapshot_for_artifact(artifact),
+        );
+        response.sheet_ref = Some(sheet_reference(sheet));
+        response.sheet_list = Some(vec![sheet.summary()]);
+        response.row_height = sheet.get_row_height(args.row_index);
+        Ok(response)
+    }
+
+    fn cleanup_and_validate_sheet(
+        &mut self,
+        request: SpreadsheetArtifactRequest,
+    ) -> Result<SpreadsheetArtifactResponse, SpreadsheetArtifactError> {
+        let args: SheetLookupArgs = parse_args(&request.action, &request.args)?;
+        let artifact_id = required_artifact_id(&request)?;
+        let artifact = self.get_artifact_mut(&artifact_id, &request.action)?;
+        let sheet_summary = {
+            let sheet = artifact.sheet_lookup_mut(
+                &request.action,
+                args.sheet_name.as_deref(),
+                args.sheet_index.map(|value| value as usize),
+            )?;
+            sheet.cleanup_and_validate_sheet()?;
+            sheet.summary()
+        };
+        let mut response = SpreadsheetArtifactResponse::new(
+            artifact_id,
+            request.action,
+            format!("Cleaned and validated `{}`", sheet_summary.name),
+            snapshot_for_artifact(artifact),
+        );
+        response.sheet_list = Some(vec![sheet_summary]);
+        Ok(response)
+    }
+
     fn get_cell(
         &mut self,
         request: SpreadsheetArtifactRequest,
@@ -1297,6 +1478,8 @@ pub struct SpreadsheetArtifactResponse {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub rendered_text: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    pub row_height: Option<f64>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub serialized_dict: Option<Value>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub serialized_json: Option<String>,
@@ -1326,6 +1509,7 @@ impl SpreadsheetArtifactResponse {
             cell_field: None,
             range: None,
             rendered_text: None,
+            row_height: None,
             serialized_dict: None,
             serialized_json: None,
             serialized_bytes_base64: None,
@@ -1414,6 +1598,44 @@ struct SetColumnWidthsArgs {
     width: f64,
 }
 
+#[derive(Debug, Deserialize)]
+struct SetColumnWidthsBulkArgs {
+    sheet_name: Option<String>,
+    sheet_index: Option<u32>,
+    widths: BTreeMap<String, f64>,
+}
+
+#[derive(Debug, Deserialize)]
+struct SetRowHeightArgs {
+    sheet_name: Option<String>,
+    sheet_index: Option<u32>,
+    row_index: u32,
+    height: Option<f64>,
+}
+
+#[derive(Debug, Deserialize)]
+struct SetRowHeightsArgs {
+    sheet_name: Option<String>,
+    sheet_index: Option<u32>,
+    start_row_index: u32,
+    end_row_index: u32,
+    height: Option<f64>,
+}
+
+#[derive(Debug, Deserialize)]
+struct SetRowHeightsBulkArgs {
+    sheet_name: Option<String>,
+    sheet_index: Option<u32>,
+    heights: BTreeMap<u32, Option<f64>>,
+}
+
+#[derive(Debug, Deserialize)]
+struct GetRowHeightArgs {
+    sheet_name: Option<String>,
+    sheet_index: Option<u32>,
+    row_index: u32,
+}
+
 #[derive(Debug, Deserialize)]
 struct CellAddressArgs {
     sheet_name: Option<String>,

codex-rs/artifact-spreadsheet/src/model.rs

@@ -18,6 +18,14 @@ use crate::parse_column_reference;
 use crate::xlsx::import_xlsx;
 use crate::xlsx::write_xlsx;
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum SpreadsheetFileType {
+    Xlsx,
+    Json,
+    Binary,
+}
+
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 #[serde(tag = "kind", content = "value", rename_all = "snake_case")]
 pub enum SpreadsheetCellValue {
@@ -683,6 +691,12 @@ impl SpreadsheetSheet {
         reference: &str,
         width: f64,
     ) -> Result<(), SpreadsheetArtifactError> {
+        if width <= 0.0 {
+            return Err(SpreadsheetArtifactError::InvalidArgs {
+                action: "set_column_widths".to_string(),
+                message: "column width must be positive".to_string(),
+            });
+        }
         let (start, end) = parse_column_reference(reference)?;
         for column in start..=end {
             self.column_widths.insert(column, width);
@@ -690,6 +704,16 @@ impl SpreadsheetSheet {
         Ok(())
     }
 
+    pub fn set_column_widths_bulk(
+        &mut self,
+        widths: &BTreeMap<String, f64>,
+    ) -> Result<(), SpreadsheetArtifactError> {
+        for (reference, width) in widths {
+            self.set_column_widths(reference, *width)?;
+        }
+        Ok(())
+    }
+
     pub fn get_column_width(
         &self,
         reference: &str,
@@ -707,6 +731,68 @@ impl SpreadsheetSheet {
             .or(self.default_column_width))
     }
 
+    pub fn set_row_height(
+        &mut self,
+        row_index: u32,
+        height: Option<f64>,
+    ) -> Result<(), SpreadsheetArtifactError> {
+        if row_index == 0 {
+            return Err(SpreadsheetArtifactError::InvalidArgs {
+                action: "set_row_height".to_string(),
+                message: "row index must be positive".to_string(),
+            });
+        }
+        if let Some(height) = height {
+            if height <= 0.0 {
+                return Err(SpreadsheetArtifactError::InvalidArgs {
+                    action: "set_row_height".to_string(),
+                    message: "row height must be positive".to_string(),
+                });
+            }
+            self.row_heights.insert(row_index, height);
+        } else {
+            self.row_heights.remove(&row_index);
+        }
+        Ok(())
+    }
+
+    pub fn set_row_heights(
+        &mut self,
+        start_row_index: u32,
+        end_row_index: u32,
+        height: Option<f64>,
+    ) -> Result<(), SpreadsheetArtifactError> {
+        if start_row_index == 0 || end_row_index == 0 {
+            return Err(SpreadsheetArtifactError::InvalidArgs {
+                action: "set_row_heights".to_string(),
+                message: "row indices must be positive".to_string(),
+            });
+        }
+        let start = start_row_index.min(end_row_index);
+        let end = start_row_index.max(end_row_index);
+        for row_index in start..=end {
+            self.set_row_height(row_index, height)?;
+        }
+        Ok(())
+    }
+
+    pub fn set_row_heights_bulk(
+        &mut self,
+        heights: &BTreeMap<u32, Option<f64>>,
+    ) -> Result<(), SpreadsheetArtifactError> {
+        for (row_index, height) in heights {
+            self.set_row_height(*row_index, *height)?;
+        }
+        Ok(())
+    }
+
+    pub fn get_row_height(&self, row_index: u32) -> Option<f64> {
+        self.row_heights
+            .get(&row_index)
+            .copied()
+            .or(self.default_row_height)
+    }
+
     pub fn cell_exists(&self, address: CellAddress) -> bool {
         self.cells.contains_key(&address)
     }
@@ -1138,6 +1224,49 @@ impl SpreadsheetSheet {
         lines.join("\n")
     }
 
+    pub fn cleanup_and_validate_sheet(&mut self) -> Result<(), SpreadsheetArtifactError> {
+        self.cells.retain(|_, cell| !cell.is_empty());
+
+        for (column, width) in &self.column_widths {
+            if *column == 0 || *width <= 0.0 {
+                return Err(SpreadsheetArtifactError::InvalidArgs {
+                    action: "cleanup_and_validate_sheet".to_string(),
+                    message: format!("invalid column width entry for column {column}"),
+                });
+            }
+        }
+        for (row, height) in &self.row_heights {
+            if *row == 0 || *height <= 0.0 {
+                return Err(SpreadsheetArtifactError::InvalidArgs {
+                    action: "cleanup_and_validate_sheet".to_string(),
+                    message: format!("invalid row height entry for row {row}"),
+                });
+            }
+        }
+
+        self.merged_ranges.sort_by_key(|range| {
+            (
+                range.start.row,
+                range.start.column,
+                range.end.row,
+                range.end.column,
+            )
+        });
+        self.merged_ranges.dedup();
+        for index in 0..self.merged_ranges.len() {
+            for other in index + 1..self.merged_ranges.len() {
+                if self.merged_ranges[index].intersects(&self.merged_ranges[other]) {
+                    return Err(SpreadsheetArtifactError::MergeConflict {
+                        action: "cleanup_and_validate_sheet".to_string(),
+                        range: self.merged_ranges[index].to_a1(),
+                        conflict: self.merged_ranges[other].to_a1(),
+                    });
+                }
+            }
+        }
+        Ok(())
+    }
+
     fn ensure_range_write_allowed(
         &self,
         range: &CellRange,
@@ -1194,6 +1323,38 @@ impl SpreadsheetArtifact {
         &["xlsx", "json", "bin"]
     }
 
+    pub fn allowed_file_mime_types() -> &'static [&'static str] {
+        &[
+            "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+            "application/json",
+            "application/octet-stream",
+        ]
+    }
+
+    pub fn allowed_file_types() -> &'static [SpreadsheetFileType] {
+        &[
+            SpreadsheetFileType::Xlsx,
+            SpreadsheetFileType::Json,
+            SpreadsheetFileType::Binary,
+        ]
+    }
+
+    pub fn get_output_file_name(
+        &self,
+        suffix: Option<&str>,
+        file_type: SpreadsheetFileType,
+    ) -> String {
+        let extension = match file_type {
+            SpreadsheetFileType::Xlsx => "xlsx",
+            SpreadsheetFileType::Json => "json",
+            SpreadsheetFileType::Binary => "bin",
+        };
+        match suffix {
+            Some(suffix) => format!("{}_{}.{}", self.artifact_id, suffix, extension),
+            None => format!("{}.{}", self.artifact_id, extension),
+        }
+    }
+
     pub fn create_sheet(
         &mut self,
         name: String,

codex-rs/artifact-spreadsheet/src/tests.rs

@@ -1,9 +1,16 @@
+use std::collections::BTreeMap;
+
 use pretty_assertions::assert_eq;
 
+use crate::CellAddress;
+use crate::CellRange;
 use crate::SpreadsheetArtifact;
 use crate::SpreadsheetArtifactManager;
 use crate::SpreadsheetArtifactRequest;
+use crate::SpreadsheetCell;
 use crate::SpreadsheetCellValue;
+use crate::SpreadsheetFileType;
+use crate::SpreadsheetSheet;
 
 #[test]
 fn manager_can_create_edit_recalculate_and_export() -> Result<(), Box<dyn std::error::Error>> {
@@ -300,3 +307,250 @@ fn manager_supports_single_value_formula_and_cite_cell_actions()
     );
     Ok(())
 }
+
+#[test]
+fn artifact_file_type_helpers_and_source_files_work() -> Result<(), Box<dyn std::error::Error>> {
+    let temp_dir = tempfile::tempdir()?;
+    let mut artifact = SpreadsheetArtifact::new(Some("Files".to_string()));
+    artifact.artifact_id = "spreadsheet_fixed".to_string();
+    artifact.create_sheet("Sheet1".to_string())?.set_value(
+        CellAddress::parse("A1")?,
+        Some(SpreadsheetCellValue::String("hello".to_string())),
+    )?;
+
+    assert_eq!(
+        SpreadsheetArtifact::allowed_file_extensions(),
+        &["xlsx", "json", "bin"]
+    );
+    assert_eq!(
+        SpreadsheetArtifact::allowed_file_mime_types(),
+        &[
+            "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+            "application/json",
+            "application/octet-stream",
+        ]
+    );
+    assert_eq!(
+        SpreadsheetArtifact::allowed_file_types().to_vec(),
+        vec![
+            SpreadsheetFileType::Xlsx,
+            SpreadsheetFileType::Json,
+            SpreadsheetFileType::Binary,
+        ]
+    );
+    assert_eq!(
+        artifact.get_output_file_name(Some("preview"), SpreadsheetFileType::Json),
+        "spreadsheet_fixed_preview.json".to_string()
+    );
+
+    let json_path = temp_dir
+        .path()
+        .join(artifact.get_output_file_name(None, SpreadsheetFileType::Json));
+    artifact.save(&json_path, Some("json"))?;
+    let restored_json = SpreadsheetArtifact::load(&json_path, None)?;
+    assert_eq!(restored_json.to_dict()?, artifact.to_dict()?);
+
+    let bytes_path = temp_dir
+        .path()
+        .join(artifact.get_output_file_name(Some("bytes"), SpreadsheetFileType::Binary));
+    artifact.save(&bytes_path, Some("bin"))?;
+    let restored_bytes = SpreadsheetArtifact::read(&bytes_path, None)?;
+    assert_eq!(restored_bytes.to_dict()?, artifact.to_dict()?);
+    Ok(())
+}
+
+#[test]
+fn sheet_cleanup_and_row_sizing_helpers_work() -> Result<(), Box<dyn std::error::Error>> {
+    let mut sheet = SpreadsheetSheet::new("Sheet1".to_string());
+    sheet.default_row_height = Some(15.0);
+    sheet.set_column_widths_bulk(&BTreeMap::from([
+        ("A".to_string(), 12.0),
+        ("C:D".to_string(), 20.0),
+    ]))?;
+    sheet.set_row_height(2, Some(18.0))?;
+    sheet.set_row_heights(3, 4, Some(22.0))?;
+    sheet.set_row_heights_bulk(&BTreeMap::from([(4, None), (5, Some(30.0))]))?;
+
+    assert_eq!(sheet.get_column_width("A")?, Some(12.0));
+    assert_eq!(sheet.get_column_width("B")?, None);
+    assert_eq!(sheet.get_column_width("D")?, Some(20.0));
+    assert_eq!(sheet.get_row_height(2), Some(18.0));
+    assert_eq!(sheet.get_row_height(3), Some(22.0));
+    assert_eq!(sheet.get_row_height(4), Some(15.0));
+    assert_eq!(sheet.get_row_height(5), Some(30.0));
+
+    sheet.cells.insert(
+        CellAddress::parse("A1")?,
+        SpreadsheetCell {
+            value: None,
+            formula: None,
+            style_index: 0,
+            citations: Vec::new(),
+        },
+    );
+    let merged = CellRange::parse("B2:C3")?;
+    sheet.merged_ranges = vec![merged.clone(), merged.clone()];
+    sheet.cleanup_and_validate_sheet()?;
+
+    assert_eq!(sheet.cells, BTreeMap::new());
+    assert_eq!(sheet.merged_ranges, vec![merged]);
+    Ok(())
+}
+
+#[test]
+fn xlsx_roundtrip_preserves_row_and_column_sizes() -> Result<(), Box<dyn std::error::Error>> {
+    let temp_dir = tempfile::tempdir()?;
+    let path = temp_dir.path().join("sizing.xlsx");
+
+    let mut artifact = SpreadsheetArtifact::new(Some("Sizing".to_string()));
+    let (expected_column_widths, expected_row_heights, expected_show_grid_lines) = {
+        let sheet = artifact.create_sheet("Sheet1".to_string())?;
+        sheet.show_grid_lines = false;
+        sheet.set_value(
+            CellAddress::parse("A1")?,
+            Some(SpreadsheetCellValue::Integer(42)),
+        )?;
+        sheet.set_column_widths_bulk(&BTreeMap::from([
+            ("A:B".to_string(), 12.5),
+            ("D".to_string(), 18.0),
+        ]))?;
+        sheet.set_row_heights_bulk(&BTreeMap::from([(2, Some(24.0)), (6, Some(19.5))]))?;
+        (
+            sheet.column_widths.clone(),
+            sheet.row_heights.clone(),
+            sheet.show_grid_lines,
+        )
+    };
+    artifact.export(&path)?;
+
+    let restored = SpreadsheetArtifact::from_source_file(&path, None)?;
+    let restored_sheet = restored.get_sheet(Some("Sheet1"), None).expect("sheet");
+    assert_eq!(restored_sheet.column_widths, expected_column_widths);
+    assert_eq!(restored_sheet.row_heights, expected_row_heights);
+    assert_eq!(restored_sheet.show_grid_lines, expected_show_grid_lines);
+    Ok(())
+}
+
+#[test]
+fn manager_supports_bulk_sizes_and_row_heights() -> Result<(), Box<dyn std::error::Error>> {
+    let temp_dir = tempfile::tempdir()?;
+    let mut manager = SpreadsheetArtifactManager::default();
+    let created = manager.execute(
+        SpreadsheetArtifactRequest {
+            artifact_id: None,
+            action: "create".to_string(),
+            args: serde_json::json!({ "name": "Sizing" }),
+        },
+        temp_dir.path(),
+    )?;
+    let artifact_id = created.artifact_id;
+
+    manager.execute(
+        SpreadsheetArtifactRequest {
+            artifact_id: Some(artifact_id.clone()),
+            action: "create_sheet".to_string(),
+            args: serde_json::json!({ "name": "Sheet1" }),
+        },
+        temp_dir.path(),
+    )?;
+
+    manager.execute(
+        SpreadsheetArtifactRequest {
+            artifact_id: Some(artifact_id.clone()),
+            action: "set_column_widths_bulk".to_string(),
+            args: serde_json::json!({
+                "sheet_name": "Sheet1",
+                "widths": {
+                    "A:B": 12.0,
+                    "D": 20.0
+                }
+            }),
+        },
+        temp_dir.path(),
+    )?;
+    manager.execute(
+        SpreadsheetArtifactRequest {
+            artifact_id: Some(artifact_id.clone()),
+            action: "set_row_height".to_string(),
+            args: serde_json::json!({
+                "sheet_name": "Sheet1",
+                "row_index": 2,
+                "height": 18.0
+            }),
+        },
+        temp_dir.path(),
+    )?;
+    manager.execute(
+        SpreadsheetArtifactRequest {
+            artifact_id: Some(artifact_id.clone()),
+            action: "set_row_heights".to_string(),
+            args: serde_json::json!({
+                "sheet_name": "Sheet1",
+                "start_row_index": 3,
+                "end_row_index": 4,
+                "height": 21.0
+            }),
+        },
+        temp_dir.path(),
+    )?;
+    manager.execute(
+        SpreadsheetArtifactRequest {
+            artifact_id: Some(artifact_id.clone()),
+            action: "set_row_heights_bulk".to_string(),
+            args: serde_json::json!({
+                "sheet_name": "Sheet1",
+                "heights": {
+                    "4": null,
+                    "5": 25.0
+                }
+            }),
+        },
+        temp_dir.path(),
+    )?;
+
+    let row_height = manager.execute(
+        SpreadsheetArtifactRequest {
+            artifact_id: Some(artifact_id.clone()),
+            action: "get_row_height".to_string(),
+            args: serde_json::json!({
+                "sheet_name": "Sheet1",
+                "row_index": 5
+            }),
+        },
+        temp_dir.path(),
+    )?;
+    assert_eq!(row_height.row_height, Some(25.0));
+
+    manager.execute(
+        SpreadsheetArtifactRequest {
+            artifact_id: Some(artifact_id.clone()),
+            action: "cleanup_and_validate_sheet".to_string(),
+            args: serde_json::json!({
+                "sheet_name": "Sheet1"
+            }),
+        },
+        temp_dir.path(),
+    )?;
+
+    let sheet = manager.execute(
+        SpreadsheetArtifactRequest {
+            artifact_id: Some(artifact_id),
+            action: "get_sheet".to_string(),
+            args: serde_json::json!({
+                "sheet_name": "Sheet1"
+            }),
+        },
+        temp_dir.path(),
+    )?;
+    let restored: SpreadsheetSheet =
+        serde_json::from_value(sheet.serialized_dict.expect("sheet dict"))?;
+    assert_eq!(
+        restored.column_widths,
+        BTreeMap::from([(1, 12.0), (2, 12.0), (4, 20.0)])
+    );
+    assert_eq!(
+        restored.row_heights,
+        BTreeMap::from([(2, 18.0), (3, 21.0), (5, 25.0)])
+    );
+    Ok(())
+}

codex-rs/artifact-spreadsheet/src/xlsx.rs

@@ -25,6 +25,9 @@ pub(crate) fn write_xlsx(
     if artifact.auto_recalculate {
         artifact.recalculate();
     }
+    for sheet in &mut artifact.sheets {
+        sheet.cleanup_and_validate_sheet()?;
+    }
 
     let file = File::create(path).map_err(|error| SpreadsheetArtifactError::ExportFailed {
         path: path.to_path_buf(),
@@ -365,7 +368,7 @@ fn parse_sheet(
         }
     }
 
-    let row_regex = Regex::new(r#"(?s)<row\b[^>]*>(.*?)</row>"#).map_err(|error| {
+    let row_regex = Regex::new(r#"(?s)<row\b([^>]*)>(.*?)</row>"#).map_err(|error| {
         SpreadsheetArtifactError::Serialization {
             message: error.to_string(),
         }
@@ -376,7 +379,20 @@ fn parse_sheet(
         }
     })?;
     for row_captures in row_regex.captures_iter(xml) {
-        let Some(row_body) = row_captures.get(1).map(|value| value.as_str()) else {
+        let row_attributes = row_captures
+            .get(1)
+            .map(|value| value.as_str())
+            .unwrap_or_default();
+        if let Some(row_index) =
+            extract_attribute(row_attributes, "r").and_then(|value| value.parse::<u32>().ok())
+            && let Some(height) =
+                extract_attribute(row_attributes, "ht").and_then(|value| value.parse::<f64>().ok())
+            && row_index > 0
+            && height > 0.0
+        {
+            sheet.row_heights.insert(row_index, height);
+        }
+        let Some(row_body) = row_captures.get(2).map(|value| value.as_str()) else {
             continue;
         };
         for cell_captures in cell_regex.captures_iter(row_body) {
@@ -628,6 +644,9 @@ fn styles_xml(artifact: &SpreadsheetArtifact) -> String {
 
 fn sheet_xml(sheet: &SpreadsheetSheet) -> String {
     let mut rows = BTreeMap::<u32, Vec<(CellAddress, &SpreadsheetCell)>>::new();
+    for row_index in sheet.row_heights.keys() {
+        rows.entry(*row_index).or_default();
+    }
     for (address, cell) in &sheet.cells {
         rows.entry(address.row).or_default().push((*address, cell));
     }
@@ -641,7 +660,12 @@ fn sheet_xml(sheet: &SpreadsheetSheet) -> String {
                 .map(|(address, cell)| cell_xml(address, cell))
                 .collect::<Vec<_>>()
                 .join("");
-            format!(r#"<row r="{row_index}">{cells}</row>"#)
+            let height = sheet
+                .row_heights
+                .get(&row_index)
+                .map(|value| format!(r#" ht="{value}" customHeight="1""#))
+                .unwrap_or_default();
+            format!(r#"<row r="{row_index}"{height}>{cells}</row>"#)
         })
         .collect::<Vec<_>>()
         .join("");