diff --git a/src/core/datamodel.ts b/src/core/datamodel.ts index 10b36e26..13f7677a 100644 --- a/src/core/datamodel.ts +++ b/src/core/datamodel.ts @@ -238,7 +238,7 @@ function stockEquationFromJson( arrayedEquation: JsonArrayedEquation | undefined, ): Equation { if (arrayedEquation) { - const dimensionNames = List(arrayedEquation.dimensions ?? []); + const dimensionNames = List(arrayedEquation.dimensions ?? []); if (arrayedEquation.elements && arrayedEquation.elements.length > 0) { return new ArrayedEquation({ dimensionNames, @@ -265,7 +265,7 @@ function auxEquationFromJson( } if (arrayedEquation) { - const dimensionNames = List(arrayedEquation.dimensions ?? []); + const dimensionNames = List(arrayedEquation.dimensions ?? []); if (arrayedEquation.elements && arrayedEquation.elements.length > 0) { return { equation: new ArrayedEquation({ diff --git a/src/libsimlin/src/lib.rs b/src/libsimlin/src/lib.rs index 2e029a50..2ba4ea55 100644 --- a/src/libsimlin/src/lib.rs +++ b/src/libsimlin/src/lib.rs @@ -3095,23 +3095,14 @@ pub unsafe extern "C" fn simlin_project_serialize_protobuf( *out_len = len; } -/// Applies a patch to the project datamodel. -/// -/// The patch is encoded as a `project_io.Patch` protobuf message. The caller can -/// request a dry run (which performs validation without committing) and control -/// whether errors are permitted. When `allow_errors` is false, any static or -/// simulation error will cause the patch to be rejected. -/// -/// On success returns `SimlinErrorCode::NoError`. On failure returns an error -/// code describing why the patch could not be applied. When `out_errors` is not /// Internal helper that applies a ProjectPatch to a project. /// -/// This is the core patch application logic shared by both protobuf and JSON entry points. -/// It handles datamodel cloning, patch application, error gathering, validation, and -/// committing changes (unless dry_run is true). +/// This is the core patch application logic. It handles datamodel cloning, +/// patch application, error gathering, validation, and committing changes +/// (unless dry_run is true). unsafe fn apply_project_patch_internal( project_ref: &SimlinProject, - patch: engine::project_io::ProjectPatch, + patch: engine::ProjectPatch, dry_run: bool, allow_errors: bool, out_collected_errors: *mut *mut SimlinError, @@ -3129,7 +3120,7 @@ unsafe fn apply_project_patch_internal( project_locked.datamodel.clone() }; - if let Err(err) = engine::apply_patch(&mut staged_datamodel, &patch) { + if let Err(err) = engine::apply_patch(&mut staged_datamodel, patch) { store_error( out_error, SimlinError::new(SimlinErrorCode::from(err.code)) @@ -3393,7 +3384,7 @@ pub unsafe extern "C" fn simlin_project_apply_patch( } }; - let patch_proto = match convert_json_project_patch(json_patch) { + let patch = match convert_json_project_patch(json_patch) { Ok(patch) => patch, Err(err) => { store_ffi_error(out_error, err); @@ -3411,7 +3402,7 @@ pub unsafe extern "C" fn simlin_project_apply_patch( apply_project_patch_internal( project_ref, - patch_proto, + patch, dry_run, allow_errors, out_collected_errors, @@ -3427,6 +3418,9 @@ enum JsonProjectOperation { #[serde(rename = "simSpecs")] sim_specs: engine::json::SimSpecs, }, + AddModel { + name: String, + }, } #[cfg_attr(feature = "debug-derive", derive(Debug))] @@ -3486,134 +3480,75 @@ enum JsonModelOperation { fn convert_json_project_patch( patch: JsonProjectPatch, -) -> Result { +) -> std::result::Result { let mut project_ops = Vec::with_capacity(patch.project_ops.len()); for op in patch.project_ops { - let converted = convert_json_project_operation(op)?; - project_ops.push(engine::project_io::ProjectOperation { - op: Some(converted), - }); + project_ops.push(convert_json_project_operation(op)?); } - let mut model_patches = Vec::with_capacity(patch.models.len()); + let mut models = Vec::with_capacity(patch.models.len()); for model in patch.models { let mut ops = Vec::with_capacity(model.ops.len()); for op in model.ops { - let converted = convert_json_model_operation(op)?; - ops.push(engine::project_io::ModelOperation { - op: Some(converted), - }); + ops.push(convert_json_model_operation(op)?); } - model_patches.push(engine::project_io::ModelPatch { + models.push(engine::ModelPatch { name: model.name, ops, }); } - Ok(engine::project_io::ProjectPatch { + Ok(engine::ProjectPatch { project_ops, - models: model_patches, + models, }) } fn convert_json_project_operation( op: JsonProjectOperation, -) -> Result { - use engine::datamodel; - use engine::project_io; - use engine::project_io::project_operation; - +) -> std::result::Result { let result = match op { JsonProjectOperation::SetSimSpecs { sim_specs } => { - let dm_sim_specs: datamodel::SimSpecs = sim_specs.into(); - let sim_specs_pb: project_io::SimSpecs = dm_sim_specs.into(); - project_operation::Op::SetSimSpecs(project_io::SetSimSpecsOp { - sim_specs: Some(sim_specs_pb), - }) + engine::ProjectOperation::SetSimSpecs(sim_specs.into()) } + JsonProjectOperation::AddModel { name } => engine::ProjectOperation::AddModel { name }, }; - Ok(result) } fn convert_json_model_operation( op: JsonModelOperation, -) -> Result { - use engine::datamodel; - use engine::project_io; - use engine::project_io::model_operation; - +) -> std::result::Result { let result = match op { - JsonModelOperation::UpsertAux { aux } => { - let dm_aux: datamodel::Aux = aux.into(); - let variable_pb = project_io::Variable::from(datamodel::Variable::Aux(dm_aux)); - let aux_pb = match variable_pb.v { - Some(project_io::variable::V::Aux(aux)) => aux, - _ => unreachable!(), - }; - model_operation::Op::UpsertAux(project_io::UpsertAuxOp { aux: Some(aux_pb) }) - } + JsonModelOperation::UpsertAux { aux } => engine::ModelOperation::UpsertAux(aux.into()), JsonModelOperation::UpsertStock { stock } => { - let dm_stock: datamodel::Stock = stock.into(); - let variable_pb = project_io::Variable::from(datamodel::Variable::Stock(dm_stock)); - let stock_pb = match variable_pb.v { - Some(project_io::variable::V::Stock(stock)) => stock, - _ => unreachable!(), - }; - model_operation::Op::UpsertStock(project_io::UpsertStockOp { - stock: Some(stock_pb), - }) - } - JsonModelOperation::UpsertFlow { flow } => { - let dm_flow: datamodel::Flow = flow.into(); - let variable_pb = project_io::Variable::from(datamodel::Variable::Flow(dm_flow)); - let flow_pb = match variable_pb.v { - Some(project_io::variable::V::Flow(flow)) => flow, - _ => unreachable!(), - }; - model_operation::Op::UpsertFlow(project_io::UpsertFlowOp { - flow: Some(flow_pb), - }) + engine::ModelOperation::UpsertStock(stock.into()) } + JsonModelOperation::UpsertFlow { flow } => engine::ModelOperation::UpsertFlow(flow.into()), JsonModelOperation::UpsertModule { module } => { - let dm_module: datamodel::Module = module.into(); - let variable_pb = project_io::Variable::from(datamodel::Variable::Module(dm_module)); - let module_pb = match variable_pb.v { - Some(project_io::variable::V::Module(module)) => module, - _ => unreachable!(), - }; - model_operation::Op::UpsertModule(project_io::UpsertModuleOp { - module: Some(module_pb), - }) + engine::ModelOperation::UpsertModule(module.into()) } JsonModelOperation::DeleteVariable { ident } => { - model_operation::Op::DeleteVariable(project_io::DeleteVariableOp { ident }) + engine::ModelOperation::DeleteVariable { ident } } JsonModelOperation::RenameVariable { from, to } => { - model_operation::Op::RenameVariable(project_io::RenameVariableOp { from, to }) - } - JsonModelOperation::UpsertView { index, view } => { - let dm_view: datamodel::View = view.into(); - let view_pb = project_io::View::from(dm_view); - model_operation::Op::UpsertView(project_io::UpsertViewOp { - index, - view: Some(view_pb), - }) - } - JsonModelOperation::DeleteView { index } => { - model_operation::Op::DeleteView(project_io::DeleteViewOp { index }) + engine::ModelOperation::RenameVariable { from, to } } + JsonModelOperation::UpsertView { index, view } => engine::ModelOperation::UpsertView { + index, + view: view.into(), + }, + JsonModelOperation::DeleteView { index } => engine::ModelOperation::DeleteView { index }, JsonModelOperation::UpdateStockFlows { ident, inflows, outflows, - } => model_operation::Op::UpdateStockFlows(project_io::UpdateStockFlowsOp { + } => engine::ModelOperation::UpdateStockFlows { ident, inflows, outflows, - }), + }, }; - Ok(result) } diff --git a/src/simlin-engine/src/lib.rs b/src/simlin-engine/src/lib.rs index 2a63f849..2dec14e5 100644 --- a/src/simlin-engine/src/lib.rs +++ b/src/simlin-engine/src/lib.rs @@ -61,7 +61,7 @@ pub mod xmile; pub use self::common::{Error, ErrorCode, ErrorKind, Result, canonicalize}; pub use self::interpreter::Simulation; pub use self::model::{ModelStage1, resolve_non_private_dependencies}; -pub use self::patch::apply_patch; +pub use self::patch::{ModelOperation, ModelPatch, ProjectOperation, ProjectPatch, apply_patch}; pub use self::project::Project; pub use self::results::{Method, Results, Specs as SimSpecs}; pub use self::variable::{Variable, identifier_set}; diff --git a/src/simlin-engine/src/patch.rs b/src/simlin-engine/src/patch.rs index 92254ae4..a1ba73df 100644 --- a/src/simlin-engine/src/patch.rs +++ b/src/simlin-engine/src/patch.rs @@ -10,76 +10,117 @@ use crate::common::{ }; use crate::datamodel::{self, Variable}; use crate::project::Project as CompiledProject; -use crate::project_io::{self, model_operation, project_operation}; -use crate::serde; use std::collections::HashMap; -macro_rules! require_field { - ($field:expr, $msg:expr) => {{ - let Some(value) = $field else { - return Err(Error::new( - ErrorKind::Model, - ErrorCode::ProtobufDecode, - Some($msg.to_string()), - )); - }; - value - }}; +/// A patch to apply to a project. Contains project-level operations +/// (like changing sim specs or adding models) and per-model patches +/// (like upserting variables or views). +pub struct ProjectPatch { + pub project_ops: Vec, + pub models: Vec, } -pub fn apply_patch( - project: &mut datamodel::Project, - patch: &project_io::ProjectPatch, -) -> Result<()> { +/// A project-level operation. +pub enum ProjectOperation { + SetSimSpecs(datamodel::SimSpecs), + SetSource(datamodel::Source), + AddModel { name: String }, +} + +/// A patch targeting a specific model within the project. +pub struct ModelPatch { + pub name: String, + pub ops: Vec, +} + +/// An operation on a single model. +pub enum ModelOperation { + UpsertStock(datamodel::Stock), + UpsertFlow(datamodel::Flow), + UpsertAux(datamodel::Aux), + UpsertModule(datamodel::Module), + DeleteVariable { + ident: String, + }, + RenameVariable { + from: String, + to: String, + }, + UpsertView { + index: u32, + view: datamodel::View, + }, + DeleteView { + index: u32, + }, + UpdateStockFlows { + ident: String, + inflows: Vec, + outflows: Vec, + }, +} + +pub fn apply_patch(project: &mut datamodel::Project, patch: ProjectPatch) -> Result<()> { let mut staged = project.clone(); // Apply project-level operations first - for project_op in &patch.project_ops { - let Some(kind) = &project_op.op else { - return Err(Error::new( - ErrorKind::Model, - ErrorCode::Generic, - Some("missing project operation".to_string()), - )); - }; - - match kind { - project_operation::Op::SetSimSpecs(specs) => apply_set_sim_specs(&mut staged, specs)?, - project_operation::Op::SetSource(op) => apply_set_source(&mut staged, op)?, + for project_op in patch.project_ops { + match project_op { + ProjectOperation::SetSimSpecs(sim_specs) => { + staged.sim_specs = sim_specs; + } + ProjectOperation::SetSource(source) => { + staged.source = Some(source); + } + ProjectOperation::AddModel { name } => { + apply_add_model(&mut staged, name)?; + } } } // Then apply model-level operations - for model_patch in &patch.models { - for op in &model_patch.ops { - let Some(kind) = &op.op else { - return Err(Error::new( - ErrorKind::Model, - ErrorCode::Generic, - Some("missing model operation".to_string()), - )); - }; - - match kind { - model_operation::Op::RenameVariable(op) => { - apply_rename_variable(&mut staged, &model_patch.name, op)? + for model_patch in patch.models { + for op in model_patch.ops { + match op { + ModelOperation::RenameVariable { from, to } => { + apply_rename_variable(&mut staged, &model_patch.name, &from, &to)?; } _ => { let model = get_model_mut(&mut staged, &model_patch.name)?; - match kind { - model_operation::Op::UpsertStock(op) => apply_upsert_stock(model, op)?, - model_operation::Op::UpsertFlow(op) => apply_upsert_flow(model, op)?, - model_operation::Op::UpsertAux(op) => apply_upsert_aux(model, op)?, - model_operation::Op::UpsertModule(op) => apply_upsert_module(model, op)?, - model_operation::Op::DeleteVariable(op) => { - apply_delete_variable(model, op)? + match op { + ModelOperation::UpsertStock(mut stock) => { + canonicalize_stock(&mut stock); + upsert_variable(model, Variable::Stock(stock)); + } + ModelOperation::UpsertFlow(mut flow) => { + canonicalize_flow(&mut flow); + upsert_variable(model, Variable::Flow(flow)); + } + ModelOperation::UpsertAux(mut aux) => { + canonicalize_aux(&mut aux); + upsert_variable(model, Variable::Aux(aux)); } - model_operation::Op::UpsertView(op) => apply_upsert_view(model, op)?, - model_operation::Op::DeleteView(op) => apply_delete_view(model, op)?, - model_operation::Op::UpdateStockFlows(op) => { - apply_update_stock_flows(model, op)? + ModelOperation::UpsertModule(mut module) => { + canonicalize_module(&mut module); + upsert_variable(model, Variable::Module(module)); } - model_operation::Op::RenameVariable(_) => unreachable!(), + ModelOperation::DeleteVariable { ident } => { + apply_delete_variable(model, &ident)?; + } + ModelOperation::UpsertView { index, view } => { + apply_upsert_view(model, index, view)?; + } + ModelOperation::DeleteView { index } => { + apply_delete_view(model, index)?; + } + ModelOperation::UpdateStockFlows { + ident, + inflows, + outflows, + } => { + apply_update_stock_flows(model, &ident, &inflows, &outflows)?; + } + ModelOperation::RenameVariable { .. } => unreachable!(), } } } @@ -141,81 +182,35 @@ fn get_model_mut<'a>( }) } -fn apply_set_sim_specs( - project: &mut datamodel::Project, - op: &project_io::SetSimSpecsOp, -) -> Result<()> { - let sim_specs = require_field!(&op.sim_specs, "missing sim_specs payload"); - - project.sim_specs.start = sim_specs.start; - project.sim_specs.stop = sim_specs.stop; - - if let Some(dt) = &sim_specs.dt { - project.sim_specs.dt = datamodel::Dt::from(*dt); - } - - if let Some(save_step) = &sim_specs.save_step { - project.sim_specs.save_step = Some(datamodel::Dt::from(*save_step)); - } else { - project.sim_specs.save_step = None; - } - - let sim_method = project_io::SimMethod::try_from(sim_specs.sim_method).unwrap_or_default(); - project.sim_specs.sim_method = datamodel::SimMethod::from(sim_method); - - if let Some(units) = &sim_specs.time_units { - if units.is_empty() { - project.sim_specs.time_units = None; - } else { - project.sim_specs.time_units = Some(units.clone()); - } - } else { - project.sim_specs.time_units = None; +fn apply_add_model(project: &mut datamodel::Project, name: String) -> Result<()> { + // Check if a model with this name already exists. + // Model names are stored and looked up as-is (no canonicalization), + // consistent with XMILE/JSON import and the C FFI simlin_project_add_model. + if project.get_model(&name).is_some() { + return Err(Error::new( + ErrorKind::Model, + ErrorCode::DuplicateVariable, + Some(format!("model '{}' already exists", name)), + )); } - - Ok(()) -} - -fn apply_upsert_stock(model: &mut datamodel::Model, op: &project_io::UpsertStockOp) -> Result<()> { - let stock_pb = require_field!(&op.stock, "missing stock payload"); - let mut stock = datamodel::Stock::from(stock_pb.clone()); - canonicalize_stock(&mut stock); - upsert_variable(model, Variable::Stock(stock)); - Ok(()) -} - -fn apply_upsert_flow(model: &mut datamodel::Model, op: &project_io::UpsertFlowOp) -> Result<()> { - let flow_pb = require_field!(&op.flow, "missing flow payload"); - let mut flow = datamodel::Flow::from(flow_pb.clone()); - canonicalize_flow(&mut flow); - upsert_variable(model, Variable::Flow(flow)); - Ok(()) -} - -fn apply_upsert_aux(model: &mut datamodel::Model, op: &project_io::UpsertAuxOp) -> Result<()> { - let aux_pb = require_field!(&op.aux, "missing auxiliary payload"); - let mut aux = datamodel::Aux::from(aux_pb.clone()); - canonicalize_aux(&mut aux); - upsert_variable(model, Variable::Aux(aux)); - Ok(()) -} - -fn apply_upsert_module( - model: &mut datamodel::Model, - op: &project_io::UpsertModuleOp, -) -> Result<()> { - let module_pb = require_field!(&op.module, "missing module payload"); - let mut module = datamodel::Module::from(module_pb.clone()); - canonicalize_module(&mut module); - upsert_variable(model, Variable::Module(module)); + project.models.push(datamodel::Model { + name, + sim_specs: None, + variables: vec![], + views: vec![], + loop_metadata: vec![], + groups: vec![], + }); Ok(()) } fn apply_update_stock_flows( model: &mut datamodel::Model, - op: &project_io::UpdateStockFlowsOp, + ident_str: &str, + inflows: &[String], + outflows: &[String], ) -> Result<()> { - let ident = canonicalize(op.ident.as_str()); + let ident = canonicalize(ident_str); let stock = model .variables @@ -232,17 +227,15 @@ fn apply_update_stock_flows( Error::new( ErrorKind::Model, ErrorCode::DoesNotExist, - Some(format!("stock '{}' not found", op.ident)), + Some(format!("stock '{}' not found", ident_str)), ) })?; - stock.inflows = op - .inflows + stock.inflows = inflows .iter() .map(|s| canonicalize(s).into_string()) .collect(); - stock.outflows = op - .outflows + stock.outflows = outflows .iter() .map(|s| canonicalize(s).into_string()) .collect(); @@ -252,11 +245,8 @@ fn apply_update_stock_flows( Ok(()) } -fn apply_delete_variable( - model: &mut datamodel::Model, - op: &project_io::DeleteVariableOp, -) -> Result<()> { - let ident = canonicalize(op.ident.as_str()); +fn apply_delete_variable(model: &mut datamodel::Model, ident_str: &str) -> Result<()> { + let ident = canonicalize(ident_str); let Some(pos) = model .variables .iter() @@ -292,10 +282,11 @@ fn apply_delete_variable( fn apply_rename_variable( project: &mut datamodel::Project, model_name: &str, - op: &project_io::RenameVariableOp, + from: &str, + to: &str, ) -> Result<()> { - let old_ident = canonicalize(op.from.as_str()); - let new_ident = canonicalize(op.to.as_str()); + let old_ident = canonicalize(from); + let new_ident = canonicalize(to); if old_ident == new_ident { return Ok(()); @@ -896,10 +887,12 @@ fn update_stock_flow_references( } } -fn apply_upsert_view(model: &mut datamodel::Model, op: &project_io::UpsertViewOp) -> Result<()> { - let view_pb = require_field!(&op.view, "missing view payload"); - let view = serde::deserialize_view(view_pb.clone()); - let index = op.index as usize; +fn apply_upsert_view( + model: &mut datamodel::Model, + index: u32, + view: datamodel::View, +) -> Result<()> { + let index = index as usize; if index < model.views.len() { model.views[index] = view; @@ -917,8 +910,8 @@ fn apply_upsert_view(model: &mut datamodel::Model, op: &project_io::UpsertViewOp } } -fn apply_delete_view(model: &mut datamodel::Model, op: &project_io::DeleteViewOp) -> Result<()> { - let index = op.index as usize; +fn apply_delete_view(model: &mut datamodel::Model, index: u32) -> Result<()> { + let index = index as usize; if index < model.views.len() { model.views.remove(index); Ok(()) @@ -931,39 +924,12 @@ fn apply_delete_view(model: &mut datamodel::Model, op: &project_io::DeleteViewOp } } -fn apply_set_source(project: &mut datamodel::Project, op: &project_io::SetSourceOp) -> Result<()> { - let source = require_field!(&op.source, "missing source payload"); - project.source = Some(datamodel::Source::from(source.clone())); - Ok(()) -} - #[cfg(test)] mod tests { use super::*; use crate::datamodel::{self, Equation, Visibility}; - use crate::project_io::variable::V; - use crate::project_io::{ - self, ModelOperation, ModelPatch, ProjectOperation, ProjectPatch, model_operation, - project_operation, - }; use crate::test_common::TestProject; - fn stock_proto(stock: datamodel::Stock) -> project_io::variable::Stock { - let variable = Variable::Stock(stock); - match project_io::Variable::from(variable).v.unwrap() { - V::Stock(stock) => stock, - _ => unreachable!(), - } - } - - fn aux_proto(aux: datamodel::Aux) -> project_io::variable::Aux { - let variable = Variable::Aux(aux); - match project_io::Variable::from(variable).v.unwrap() { - V::Aux(aux) => aux, - _ => unreachable!(), - } - } - #[test] fn upsert_aux_adds_variable() { let mut project = TestProject::new("test").build_datamodel(); @@ -982,15 +948,11 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::UpsertAux(project_io::UpsertAuxOp { - aux: Some(aux_proto(aux.clone())), - })), - }], + ops: vec![ModelOperation::UpsertAux(aux.clone())], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); let var = model.get_variable("new_aux").unwrap(); match var { @@ -1004,7 +966,7 @@ mod tests { let mut project = TestProject::new("test") .stock("stock", "1", &[], &[], None) .build_datamodel(); - let mut stock = datamodel::Stock { + let stock = datamodel::Stock { ident: "stock".to_string(), equation: Equation::Scalar("5".to_string(), None), documentation: "docs".to_string(), @@ -1017,22 +979,15 @@ mod tests { ai_state: None, uid: Some(10), }; - stock.inflows.sort(); let patch = ProjectPatch { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::UpsertStock( - project_io::UpsertStockOp { - stock: Some(stock_proto(stock.clone())), - }, - )), - }], + ops: vec![ModelOperation::UpsertStock(stock.clone())], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); let var = model.get_variable("stock").unwrap(); match var { @@ -1056,17 +1011,13 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::DeleteVariable( - project_io::DeleteVariableOp { - ident: "flow".to_string(), - }, - )), + ops: vec![ModelOperation::DeleteVariable { + ident: "flow".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); assert!(model.get_variable("flow").is_none()); match model.get_variable("stock").unwrap() { @@ -1088,18 +1039,14 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::RenameVariable( - project_io::RenameVariableOp { - from: "flow".to_string(), - to: "new_flow".to_string(), - }, - )), + ops: vec![ModelOperation::RenameVariable { + from: "flow".to_string(), + to: "new_flow".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); assert!(model.get_variable("flow").is_none()); match model.get_variable("new_flow").unwrap() { @@ -1116,30 +1063,22 @@ mod tests { } #[test] - fn set_sim_specs_partial_update() { + fn set_sim_specs() { let mut project = TestProject::new("test").build_datamodel(); + let new_specs = datamodel::SimSpecs { + start: 5.0, + stop: project.sim_specs.stop, + dt: datamodel::Dt::Dt(0.5), + save_step: None, + sim_method: datamodel::SimMethod::RungeKutta4, + time_units: Some("days".to_string()), + }; let patch = ProjectPatch { - project_ops: vec![ProjectOperation { - op: Some(project_operation::Op::SetSimSpecs( - project_io::SetSimSpecsOp { - sim_specs: Some(project_io::SimSpecs { - start: 5.0, - stop: project.sim_specs.stop, - dt: Some(project_io::Dt { - value: 0.5, - is_reciprocal: false, - }), - save_step: None, - sim_method: project_io::SimMethod::RungeKutta4 as i32, - time_units: Some("days".to_string()), - }), - }, - )), - }], + project_ops: vec![ProjectOperation::SetSimSpecs(new_specs)], models: vec![], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); assert_eq!(project.sim_specs.start, 5.0); assert_eq!(project.sim_specs.dt, datamodel::Dt::Dt(0.5)); assert!(project.sim_specs.save_step.is_none()); @@ -1153,27 +1092,24 @@ mod tests { #[test] fn upsert_view_and_delete() { let mut project = TestProject::new("test").build_datamodel(); - let view = project_io::View { - kind: project_io::view::ViewType::StockFlow as i32, + let view = datamodel::View::StockFlow(datamodel::StockFlow { elements: vec![], - view_box: None, + view_box: datamodel::Rect::default(), zoom: 1.0, use_lettered_polarity: false, - }; + }); let patch = ProjectPatch { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::UpsertView(project_io::UpsertViewOp { - index: 0, - view: Some(view.clone()), - })), + ops: vec![ModelOperation::UpsertView { + index: 0, + view: view.clone(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); assert_eq!(model.views.len(), 1); @@ -1181,15 +1117,11 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::DeleteView(project_io::DeleteViewOp { - index: 0, - })), - }], + ops: vec![ModelOperation::DeleteView { index: 0 }], }], }; - apply_patch(&mut project, &delete_patch).unwrap(); + apply_patch(&mut project, delete_patch).unwrap(); let model = project.get_model("main").unwrap(); assert!(model.views.is_empty()); } @@ -1198,18 +1130,14 @@ mod tests { fn set_source() { let mut project = TestProject::new("test").build_datamodel(); let patch = ProjectPatch { - project_ops: vec![ProjectOperation { - op: Some(project_operation::Op::SetSource(project_io::SetSourceOp { - source: Some(project_io::Source { - extension: project_io::source::Extension::Xmile as i32, - content: "hello".to_string(), - }), - })), - }], + project_ops: vec![ProjectOperation::SetSource(datamodel::Source { + extension: datamodel::Extension::Xmile, + content: "hello".to_string(), + })], models: vec![], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); assert!(project.source.is_some()); assert_eq!(project.source.as_ref().unwrap().content, "hello"); } @@ -1224,18 +1152,14 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::RenameVariable( - project_io::RenameVariableOp { - from: "flow".to_string(), - to: "flow2".to_string(), - }, - )), + ops: vec![ModelOperation::RenameVariable { + from: "flow".to_string(), + to: "flow2".to_string(), }], }], }; - let err = apply_patch(&mut project, &patch).unwrap_err(); + let err = apply_patch(&mut project, patch).unwrap_err(); assert_eq!(err.code, ErrorCode::DuplicateVariable); assert_eq!(err.kind, ErrorKind::Model); } @@ -1251,18 +1175,14 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::RenameVariable( - project_io::RenameVariableOp { - from: "bar".to_string(), - to: "baz".to_string(), - }, - )), + ops: vec![ModelOperation::RenameVariable { + from: "bar".to_string(), + to: "baz".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); match model.get_variable("foo").unwrap() { @@ -1337,18 +1257,14 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::RenameVariable( - project_io::RenameVariableOp { - from: "input".to_string(), - to: "new_input".to_string(), - }, - )), + ops: vec![ModelOperation::RenameVariable { + from: "input".to_string(), + to: "new_input".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); match model.get_variable("consumer").unwrap() { @@ -1427,18 +1343,14 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::RenameVariable( - project_io::RenameVariableOp { - from: "foo".to_string(), - to: "renamed_foo".to_string(), - }, - )), + ops: vec![ModelOperation::RenameVariable { + from: "foo".to_string(), + to: "renamed_foo".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); match model.get_variable("consumer").unwrap() { @@ -1473,18 +1385,14 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::RenameVariable( - project_io::RenameVariableOp { - from: "foo".to_string(), - to: "bar".to_string(), - }, - )), + ops: vec![ModelOperation::RenameVariable { + from: "foo".to_string(), + to: "bar".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); match model.get_variable("consumer").unwrap() { @@ -1563,18 +1471,14 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::RenameVariable( - project_io::RenameVariableOp { - from: "base_value".to_string(), - to: "initial_value".to_string(), - }, - )), + ops: vec![ModelOperation::RenameVariable { + from: "base_value".to_string(), + to: "initial_value".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); match model.get_variable("regional_growth").unwrap() { @@ -1656,18 +1560,14 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::RenameVariable( - project_io::RenameVariableOp { - from: "price".to_string(), - to: "unit_price".to_string(), - }, - )), + ops: vec![ModelOperation::RenameVariable { + from: "price".to_string(), + to: "unit_price".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); match model.get_variable("revenue").unwrap() { @@ -1693,18 +1593,14 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::RenameVariable( - project_io::RenameVariableOp { - from: "initial_stock".to_string(), - to: "starting_inventory".to_string(), - }, - )), + ops: vec![ModelOperation::RenameVariable { + from: "initial_stock".to_string(), + to: "starting_inventory".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); match model.get_variable("inventory").unwrap() { @@ -1756,17 +1652,11 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::UpsertStock( - project_io::UpsertStockOp { - stock: Some(stock_proto(stock.clone())), - }, - )), - }], + ops: vec![ModelOperation::UpsertStock(stock.clone())], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); let var = model.get_variable("inventory").unwrap(); @@ -1793,32 +1683,25 @@ mod tests { _ => panic!("expected stock"), } - // Apply updateStockFlows to remove the inflow let patch = ProjectPatch { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::UpdateStockFlows( - project_io::UpdateStockFlowsOp { - ident: "inventory".to_string(), - inflows: vec![], - outflows: vec![], - }, - )), + ops: vec![ModelOperation::UpdateStockFlows { + ident: "inventory".to_string(), + inflows: vec![], + outflows: vec![], }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); match model.get_variable("inventory").unwrap() { Variable::Stock(stock) => { - // Flows should be updated assert!(stock.inflows.is_empty()); assert!(stock.outflows.is_empty()); - // Equation should be preserved assert_eq!(stock.equation, Equation::Scalar("100".to_string(), None)); } _ => panic!("expected stock"), @@ -1836,39 +1719,32 @@ mod tests { &[], Some("people"), "Total population", - true, // non_negative - true, // can_be_module_input + true, + true, Visibility::Public, Some(42), ) .build_datamodel(); - // Disconnect the flow let patch = ProjectPatch { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::UpdateStockFlows( - project_io::UpdateStockFlowsOp { - ident: "population".to_string(), - inflows: vec![], - outflows: vec![], - }, - )), + ops: vec![ModelOperation::UpdateStockFlows { + ident: "population".to_string(), + inflows: vec![], + outflows: vec![], }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); match model.get_variable("population").unwrap() { Variable::Stock(stock) => { - // Flows updated assert!(stock.inflows.is_empty()); assert!(stock.outflows.is_empty()); - // All other fields preserved assert_eq!(stock.equation, Equation::Scalar("1000".to_string(), None)); assert_eq!(stock.documentation, "Total population"); assert_eq!(stock.units, Some("people".to_string())); @@ -1889,19 +1765,15 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::UpdateStockFlows( - project_io::UpdateStockFlowsOp { - ident: "nonexistent".to_string(), - inflows: vec![], - outflows: vec![], - }, - )), + ops: vec![ModelOperation::UpdateStockFlows { + ident: "nonexistent".to_string(), + inflows: vec![], + outflows: vec![], }], }], }; - let err = apply_patch(&mut project, &patch).unwrap_err(); + let err = apply_patch(&mut project, patch).unwrap_err(); assert_eq!(err.code, ErrorCode::DoesNotExist); } @@ -1930,18 +1802,14 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::RenameVariable( - project_io::RenameVariableOp { - from: "alpha".to_string(), - to: "gamma".to_string(), - }, - )), + ops: vec![ModelOperation::RenameVariable { + from: "alpha".to_string(), + to: "gamma".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); assert_eq!(model.groups.len(), 1); @@ -1973,20 +1841,525 @@ mod tests { project_ops: vec![], models: vec![ModelPatch { name: "main".to_string(), - ops: vec![ModelOperation { - op: Some(model_operation::Op::DeleteVariable( - project_io::DeleteVariableOp { - ident: "alpha".to_string(), - }, - )), + ops: vec![ModelOperation::DeleteVariable { + ident: "alpha".to_string(), }], }], }; - apply_patch(&mut project, &patch).unwrap(); + apply_patch(&mut project, patch).unwrap(); let model = project.get_model("main").unwrap(); assert_eq!(model.groups.len(), 1); assert_eq!(model.groups[0].members, vec!["beta"]); } + + // --- New tests for module support and AddModel --- + + #[test] + fn add_model_creates_empty_model() { + let mut project = TestProject::new("test").build_datamodel(); + assert_eq!(project.models.len(), 1); + + let patch = ProjectPatch { + project_ops: vec![ProjectOperation::AddModel { + name: "submodel".to_string(), + }], + models: vec![], + }; + + apply_patch(&mut project, patch).unwrap(); + assert_eq!(project.models.len(), 2); + let submodel = project.get_model("submodel").unwrap(); + assert!(submodel.variables.is_empty()); + assert!(submodel.views.is_empty()); + } + + #[test] + fn add_model_duplicate_returns_error() { + let mut project = TestProject::new("test").build_datamodel(); + + let patch = ProjectPatch { + project_ops: vec![ProjectOperation::AddModel { + name: "main".to_string(), + }], + models: vec![], + }; + + let err = apply_patch(&mut project, patch).unwrap_err(); + assert_eq!(err.code, ErrorCode::DuplicateVariable); + } + + #[test] + fn upsert_module_adds_module_variable() { + let mut project = TestProject::new("test").build_datamodel(); + + // First add the submodel + project.models.push(datamodel::Model { + name: "submodel".to_string(), + sim_specs: None, + variables: vec![datamodel::Variable::Aux(datamodel::Aux { + ident: "output".to_string(), + equation: Equation::Scalar("42".to_string(), None), + documentation: String::new(), + units: None, + gf: None, + can_be_module_input: false, + visibility: Visibility::Public, + ai_state: None, + uid: None, + })], + views: vec![], + loop_metadata: vec![], + groups: vec![], + }); + + let module = datamodel::Module { + ident: "my_module".to_string(), + model_name: "submodel".to_string(), + documentation: "A test module".to_string(), + units: None, + references: vec![], + can_be_module_input: false, + visibility: Visibility::Private, + ai_state: None, + uid: Some(100), + }; + + let patch = ProjectPatch { + project_ops: vec![], + models: vec![ModelPatch { + name: "main".to_string(), + ops: vec![ModelOperation::UpsertModule(module.clone())], + }], + }; + + apply_patch(&mut project, patch).unwrap(); + let model = project.get_model("main").unwrap(); + match model.get_variable("my_module").unwrap() { + Variable::Module(m) => { + assert_eq!(m.model_name, "submodel"); + assert_eq!(m.documentation, "A test module"); + assert_eq!(m.uid, Some(100)); + } + _ => panic!("expected module"), + } + } + + #[test] + fn upsert_module_with_references() { + let mut project = TestProject::new("test") + .aux("local_input", "10", None) + .build_datamodel(); + + project.models.push(datamodel::Model { + name: "submodel".to_string(), + sim_specs: None, + variables: vec![datamodel::Variable::Aux(datamodel::Aux { + ident: "input_var".to_string(), + equation: Equation::Scalar("0".to_string(), None), + documentation: String::new(), + units: None, + gf: None, + can_be_module_input: true, + visibility: Visibility::Public, + ai_state: None, + uid: None, + })], + views: vec![], + loop_metadata: vec![], + groups: vec![], + }); + + let module = datamodel::Module { + ident: "my_module".to_string(), + model_name: "submodel".to_string(), + documentation: String::new(), + units: None, + references: vec![datamodel::ModuleReference { + src: "local_input".to_string(), + dst: "input_var".to_string(), + }], + can_be_module_input: false, + visibility: Visibility::Private, + ai_state: None, + uid: None, + }; + + let patch = ProjectPatch { + project_ops: vec![], + models: vec![ModelPatch { + name: "main".to_string(), + ops: vec![ModelOperation::UpsertModule(module)], + }], + }; + + apply_patch(&mut project, patch).unwrap(); + let model = project.get_model("main").unwrap(); + match model.get_variable("my_module").unwrap() { + Variable::Module(m) => { + assert_eq!(m.references.len(), 1); + assert_eq!(m.references[0].src, "local_input"); + assert_eq!(m.references[0].dst, "input_var"); + } + _ => panic!("expected module"), + } + } + + #[test] + fn upsert_module_replaces_existing() { + let mut project = TestProject::new("test").build_datamodel(); + project.models.push(datamodel::Model { + name: "submodel".to_string(), + sim_specs: None, + variables: vec![], + views: vec![], + loop_metadata: vec![], + groups: vec![], + }); + + // Add initial module + let initial_module = datamodel::Module { + ident: "my_module".to_string(), + model_name: "submodel".to_string(), + documentation: "initial".to_string(), + units: None, + references: vec![], + can_be_module_input: false, + visibility: Visibility::Private, + ai_state: None, + uid: Some(1), + }; + let patch1 = ProjectPatch { + project_ops: vec![], + models: vec![ModelPatch { + name: "main".to_string(), + ops: vec![ModelOperation::UpsertModule(initial_module)], + }], + }; + apply_patch(&mut project, patch1).unwrap(); + + // Now upsert with updated data + let updated_module = datamodel::Module { + ident: "my_module".to_string(), + model_name: "submodel".to_string(), + documentation: "updated".to_string(), + units: Some("widgets".to_string()), + references: vec![], + can_be_module_input: true, + visibility: Visibility::Public, + ai_state: None, + uid: Some(1), + }; + let patch2 = ProjectPatch { + project_ops: vec![], + models: vec![ModelPatch { + name: "main".to_string(), + ops: vec![ModelOperation::UpsertModule(updated_module)], + }], + }; + apply_patch(&mut project, patch2).unwrap(); + + let model = project.get_model("main").unwrap(); + match model.get_variable("my_module").unwrap() { + Variable::Module(m) => { + assert_eq!(m.documentation, "updated"); + assert_eq!(m.units, Some("widgets".to_string())); + assert!(m.can_be_module_input); + assert_eq!(m.visibility, Visibility::Public); + } + _ => panic!("expected module"), + } + } + + #[test] + fn delete_module_variable() { + let mut project = TestProject::new("test").build_datamodel(); + project.models.push(datamodel::Model { + name: "submodel".to_string(), + sim_specs: None, + variables: vec![], + views: vec![], + loop_metadata: vec![], + groups: vec![], + }); + + let module = datamodel::Module { + ident: "my_module".to_string(), + model_name: "submodel".to_string(), + documentation: String::new(), + units: None, + references: vec![], + can_be_module_input: false, + visibility: Visibility::Private, + ai_state: None, + uid: None, + }; + let add_patch = ProjectPatch { + project_ops: vec![], + models: vec![ModelPatch { + name: "main".to_string(), + ops: vec![ModelOperation::UpsertModule(module)], + }], + }; + apply_patch(&mut project, add_patch).unwrap(); + assert!( + project + .get_model("main") + .unwrap() + .get_variable("my_module") + .is_some() + ); + + let delete_patch = ProjectPatch { + project_ops: vec![], + models: vec![ModelPatch { + name: "main".to_string(), + ops: vec![ModelOperation::DeleteVariable { + ident: "my_module".to_string(), + }], + }], + }; + apply_patch(&mut project, delete_patch).unwrap(); + assert!( + project + .get_model("main") + .unwrap() + .get_variable("my_module") + .is_none() + ); + } + + #[test] + fn add_model_and_module_in_same_patch() { + let mut project = TestProject::new("test") + .aux("driver", "100", None) + .build_datamodel(); + + let module = datamodel::Module { + ident: "sub".to_string(), + model_name: "new_submodel".to_string(), + documentation: String::new(), + units: None, + references: vec![datamodel::ModuleReference { + src: "driver".to_string(), + dst: "input".to_string(), + }], + can_be_module_input: false, + visibility: Visibility::Private, + ai_state: None, + uid: None, + }; + + let patch = ProjectPatch { + project_ops: vec![ProjectOperation::AddModel { + name: "new_submodel".to_string(), + }], + models: vec![ + // Add a variable to the new submodel + ModelPatch { + name: "new_submodel".to_string(), + ops: vec![ModelOperation::UpsertAux(datamodel::Aux { + ident: "input".to_string(), + equation: Equation::Scalar("0".to_string(), None), + documentation: String::new(), + units: None, + gf: None, + can_be_module_input: true, + visibility: Visibility::Public, + ai_state: None, + uid: None, + })], + }, + // Add the module reference to main + ModelPatch { + name: "main".to_string(), + ops: vec![ModelOperation::UpsertModule(module)], + }, + ], + }; + + apply_patch(&mut project, patch).unwrap(); + + // Verify submodel was created with variable + let submodel = project.get_model("new_submodel").unwrap(); + assert!(submodel.get_variable("input").is_some()); + + // Verify module was added to main + let main = project.get_model("main").unwrap(); + match main.get_variable("sub").unwrap() { + Variable::Module(m) => { + assert_eq!(m.model_name, "new_submodel"); + assert_eq!(m.references.len(), 1); + } + _ => panic!("expected module"), + } + } + + #[test] + fn rename_variable_updates_module_references_in_same_model() { + let mut project = TestProject::new("test") + .aux("old_name", "42", None) + .build_datamodel(); + + project.models.push(datamodel::Model { + name: "submodel".to_string(), + sim_specs: None, + variables: vec![datamodel::Variable::Aux(datamodel::Aux { + ident: "sub_input".to_string(), + equation: Equation::Scalar("0".to_string(), None), + documentation: String::new(), + units: None, + gf: None, + can_be_module_input: true, + visibility: Visibility::Public, + ai_state: None, + uid: None, + })], + views: vec![], + loop_metadata: vec![], + groups: vec![], + }); + + // Add a module that references old_name + let module = datamodel::Module { + ident: "child".to_string(), + model_name: "submodel".to_string(), + documentation: String::new(), + units: None, + references: vec![datamodel::ModuleReference { + src: "old_name".to_string(), + dst: "self.sub_input".to_string(), + }], + can_be_module_input: false, + visibility: Visibility::Private, + ai_state: None, + uid: None, + }; + let add_module_patch = ProjectPatch { + project_ops: vec![], + models: vec![ModelPatch { + name: "main".to_string(), + ops: vec![ModelOperation::UpsertModule(module)], + }], + }; + apply_patch(&mut project, add_module_patch).unwrap(); + + // Now rename old_name to new_name + let rename_patch = ProjectPatch { + project_ops: vec![], + models: vec![ModelPatch { + name: "main".to_string(), + ops: vec![ModelOperation::RenameVariable { + from: "old_name".to_string(), + to: "new_name".to_string(), + }], + }], + }; + apply_patch(&mut project, rename_patch).unwrap(); + + let model = project.get_model("main").unwrap(); + let module = model + .variables + .iter() + .find_map(|v| match v { + Variable::Module(m) => Some(m), + _ => None, + }) + .unwrap(); + assert_eq!(module.references[0].src, "new_name"); + } + + #[test] + fn patch_rollback_on_error() { + let mut project = TestProject::new("test") + .aux("x", "1", None) + .build_datamodel(); + + // Try a patch that adds a variable then operates on a nonexistent model + let patch = ProjectPatch { + project_ops: vec![], + models: vec![ + ModelPatch { + name: "main".to_string(), + ops: vec![ModelOperation::UpsertAux(datamodel::Aux { + ident: "y".to_string(), + equation: Equation::Scalar("2".to_string(), None), + documentation: String::new(), + units: None, + gf: None, + can_be_module_input: false, + visibility: Visibility::Private, + ai_state: None, + uid: None, + })], + }, + ModelPatch { + name: "nonexistent_model".to_string(), + ops: vec![ModelOperation::DeleteVariable { + ident: "z".to_string(), + }], + }, + ], + }; + + let result = apply_patch(&mut project, patch); + assert!(result.is_err()); + + // Project should be unchanged (rollback) + let model = project.get_model("main").unwrap(); + assert!( + model.get_variable("y").is_none(), + "y should not have been added on error" + ); + assert!(model.get_variable("x").is_some(), "x should still exist"); + } + + #[test] + fn add_model_preserves_display_name() { + let mut project = TestProject::new("test").build_datamodel(); + + let patch = ProjectPatch { + project_ops: vec![ProjectOperation::AddModel { + name: "Customer Growth".to_string(), + }], + models: vec![], + }; + + apply_patch(&mut project, patch).unwrap(); + assert_eq!(project.models.len(), 2); + // The model should be stored with its display name, not canonicalized + assert_eq!(project.models[1].name, "Customer Growth"); + // And we should be able to find it by its display name + assert!(project.get_model("Customer Growth").is_some()); + } + + #[test] + fn add_model_and_operate_on_it_in_same_patch_with_display_name() { + let mut project = TestProject::new("test").build_datamodel(); + + let patch = ProjectPatch { + project_ops: vec![ProjectOperation::AddModel { + name: "Customer Growth".to_string(), + }], + models: vec![ModelPatch { + name: "Customer Growth".to_string(), + ops: vec![ModelOperation::UpsertAux(datamodel::Aux { + ident: "growth_rate".to_string(), + equation: Equation::Scalar("0.05".to_string(), None), + documentation: String::new(), + units: None, + gf: None, + can_be_module_input: false, + visibility: Visibility::Private, + ai_state: None, + uid: None, + })], + }], + }; + + apply_patch(&mut project, patch).unwrap(); + let model = project.get_model("Customer Growth").unwrap(); + assert!(model.get_variable("growth_rate").is_some()); + } }