From 7dd4043afecf6fef8f0c50b8fc4c13fbb91ef15a Mon Sep 17 00:00:00 2001 From: jyviko <2501648+jyviko@users.noreply.github.com> Date: Thu, 15 Oct 2026 08:38:22 -0400 Subject: [PATCH] engine: refuse unknown command parameters, suggest a valid name, document heat loads and thermal materials Every command with a schema now refuses a top-level key it does not take, naming the key and the accepted ones (#316). A bad definition name's error suggests the nearest valid one (#320). The load and material params docs list the heat loads and the thermal block. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UhhZMBTviDQxZvwnyP89fo --- crates/engine/src/cmd/define.rs | 4 +-- crates/engine/src/lib.rs | 16 +++++++++- crates/engine/src/schema.rs | 55 +++++++++++++++++++++++++++++++++ crates/model/src/lib.rs | 36 ++++++++++++++++++++- 4 files changed, 107 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/define.rs b/crates/engine/src/cmd/define.rs index df3158f..83671e3 100644 --- a/crates/engine/src/cmd/define.rs +++ b/crates/engine/src/cmd/define.rs @@ -92,7 +92,7 @@ definition_commands!( Material, "material", "Material", - r#"{"name": "Steel", "elastic": {"type": "isotropic", "young": 210000, "poisson": 0.3} | {"type": "engineering_constants", "young": [E1, E2, E3], "poisson": [ν12, ν13, ν23], "shear": [G12, G13, G23]}, "density": 7.85e-9, "fluid": {"type": "newtonian", "viscosity": 1.0e-9} (dynamic viscosity; a fluid needs a density), "plastic": {…} (see model.material.plasticity), "damage": {…}, "erosion": {…} (see model.material.damage), "hyperelastic": {"type": "neo_hookean"} | {"type": "mooney_rivlin", "c01": 0.1} (C10 = μ/2 − C01) | {"type": "yeoh", "c20": -0.01, "c30": 0.001} (C10 = μ/2) (large strain in nonlinear steps, from the isotropic constants: μ = E/(2(1 + ν)), κ = E/(3(1 − 2ν)); without it an elastic material is St Venant–Kirchhoff there)} — in the model's unit system (model.units), never converted"#, + r#"{"name": "Steel", "elastic": {"type": "isotropic", "young": 210000, "poisson": 0.3} | {"type": "engineering_constants", "young": [E1, E2, E3], "poisson": [ν12, ν13, ν23], "shear": [G12, G13, G23]}, "density": 7.85e-9, "fluid": {"type": "newtonian", "viscosity": 1.0e-9} (dynamic viscosity; a fluid needs a density), "plastic": {…} (see model.material.plasticity), "damage": {…}, "erosion": {…} (see model.material.damage), "hyperelastic": {"type": "neo_hookean"} | {"type": "mooney_rivlin", "c01": 0.1} (C10 = μ/2 − C01) | {"type": "yeoh", "c20": -0.01, "c30": 0.001} (C10 = μ/2) (large strain in nonlinear steps, from the isotropic constants: μ = E/(2(1 + ν)), κ = E/(3(1 − 2ν)); without it an elastic material is St Venant–Kirchhoff there), "thermal": {"conductivity": 0.05 | [kx, ky, kz], "specific_heat": 4.6e8, "expansion": 1.2e-5 | [αx, αy, αz], "reference_temperature": 20 (zero thermal strain; default 0)} (heat transfer and thermal strain; each key optional)} — in the model's unit system (model.units), never converted"#, super::forms::MATERIAL, super::forms::MATERIAL_DELETE, add_material, @@ -267,7 +267,7 @@ definition_commands!( Load, "load", "Load", - r#"{"name": "push", "type": "force|moment", "target": {"set": "…"} | {"reference_point": "…"}, "vector": [x, y, z], "distribution": "total" (default: the vector is the total, split by area over the set's faces, or equally over its nodes) | "per_node" (at every node: depends on the mesh)} | {"name", "type": "traction", "set", "vector" (per area, on the set's faces)} | {"name", "type": "pressure", "set", "magnitude" (positive pushes in)} | {"name", "type": "gravity", "acceleration": [x, y, z], "set" (optional, default the whole model)} | {"name", "type": "body_force", "set", "vector" (per volume)} | {"name", "type": "centrifugal", "angular_velocity" (rad per time unit), "point": [x, y, z] (on the axis), "axis": [x, y, z], "set" (optional)} — 2D models: in-plane vectors, surface loads on element edges; active in the steps that list it (model.step.activate)"#, + r#"{"name": "push", "type": "force|moment", "target": {"set": "…"} | {"reference_point": "…"}, "vector": [x, y, z], "distribution": "total" (default: the vector is the total, split by area over the set's faces, or equally over its nodes) | "per_node" (at every node: depends on the mesh)} | {"name", "type": "traction", "set", "vector" (per area, on the set's faces)} | {"name", "type": "pressure", "set", "magnitude" (positive pushes in)} | {"name", "type": "gravity", "acceleration": [x, y, z], "set" (optional, default the whole model)} | {"name", "type": "body_force", "set", "vector" (per volume)} | {"name", "type": "centrifugal", "angular_velocity" (rad per time unit), "point": [x, y, z] (on the axis), "axis": [x, y, z], "set" (optional)} | {"name", "type": "surface_flux", "set", "flux" (heat per area into the set's faces)} | {"name", "type": "convection", "set", "film" (film coefficient h), "sink" (surrounding temperature): heat flux h (sink − T) into the set's faces} | {"name", "type": "heat_source", "set", "power" (heat per volume in the set's elements)} | {"name", "type": "temperature", "target": {"set": "…"} | {"reference_point": "…"}, "value" (a temperature that strains the material in stress steps)} — heat loads act in heat transfer and coupled steps; 2D models: in-plane vectors, surface loads on element edges; active in the steps that list it (model.step.activate)"#, super::forms::LOAD, super::forms::LOAD_DELETE, add_load, diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 47c8f4b..69a7aa5 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -171,6 +171,16 @@ pub fn find_command(id: &str) -> Option<&'static CommandSpec> { commands().iter().find(|c| c.id == id) } +/// Refuse a parameter the command does not take, naming the ones it does: a script that sets +/// `deformed` on `results.contour` learns it belongs elsewhere instead of being ignored. +fn check_keys(spec: &CommandSpec, params: &Value) -> Result<()> { + match spec.form.and_then(|form| schema::unknown_key(form, params)) { + Some((key, accepted)) if accepted.is_empty() => Err(EngineError::Params(format!("{} takes no parameters (got `{key}`)", spec.id))), + Some((key, accepted)) => Err(EngineError::Params(format!("{} has no parameter `{key}`; it takes {}", spec.id, accepted.join(", ")))), + None => Ok(()), + } +} + /// A new revision number, unique in the process: every state of every session has its own. pub(crate) fn next_revision() -> u64 { static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); @@ -269,7 +279,11 @@ impl Session { let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.chars().take(100).collect()))?; let timer = usage::Timer::start(self.origin); let elements = self.model.elements().len(); - let result = self.availability(spec).map_err(EngineError::Unavailable).and_then(|()| self.run_command(spec, params)); + let result = self + .availability(spec) + .map_err(EngineError::Unavailable) + .and_then(|()| check_keys(spec, params)) + .and_then(|()| self.run_command(spec, params)); if timer.counting() { let milestone = usage::milestone(spec.id, (elements, self.model.elements().len())); let sample = diff --git a/crates/engine/src/schema.rs b/crates/engine/src/schema.rs index bc43888..0081250 100644 --- a/crates/engine/src/schema.rs +++ b/crates/engine/src/schema.rs @@ -777,6 +777,38 @@ pub fn object_problems(fields: &'static [Field], obj: &Map) -> Ve out } +/// The top-level keys `params` may hold for these fields: each field's key, and for a flattened +/// one-of (a load's `type`) its tag and the fields of the variant `params` picks (of every +/// variant when it picks none). +pub fn accepted_keys(fields: &[Field], params: &Value) -> Vec<&'static str> { + let mut keys = Vec::new(); + for f in fields { + match f.kind { + Kind::Variants { tag, variants } if f.key.is_empty() => { + keys.push(tag); + let picked = params.get(tag).and_then(Value::as_str).and_then(|t| variants.iter().find(|v| v.tag == t)); + for v in variants.iter().filter(|v| picked.is_none_or(|p| p.tag == v.tag)) { + for key in accepted_keys(v.fields, params) { + if !keys.contains(&key) { + keys.push(key); + } + } + } + } + _ => keys.push(f.key), + } + } + keys +} + +/// The first top-level key of `params` that `fields` do not describe, with the keys they do: +/// a misspelt or misplaced parameter is refused rather than silently ignored. +pub fn unknown_key(fields: &[Field], params: &Value) -> Option<(String, Vec<&'static str>)> { + let obj = params.as_object()?; + let accepted = accepted_keys(fields, params); + obj.keys().find(|k| !accepted.contains(&k.as_str())).map(|k| (k.chars().take(64).collect(), accepted)) +} + /// The path of field `key` inside `prefix` (`elastic/young`). pub fn join(prefix: &str, key: &str) -> String { if prefix.is_empty() { @@ -1106,4 +1138,27 @@ mod tests { assert_eq!(s["oneOf"][1]["properties"]["type"]["const"], "pressure"); assert_eq!(s["oneOf"][1]["required"], json!(["magnitude", "set", "type"])); } + + #[test] + fn unknown_keys_are_found_per_variant() { + assert_eq!(unknown_key(LOADISH, &json!({"name": "P", "type": "pressure", "magnitude": 1})), None); + let (key, accepted) = unknown_key(LOADISH, &json!({"name": "P", "type": "pressure", "magnitud": 1})).unwrap(); + assert_eq!(key, "magnitud"); + assert!(accepted.contains(&"magnitude") && accepted.contains(&"name")); + } + + #[test] + fn every_command_refuses_an_unknown_parameter() { + for spec in crate::commands() { + let mut s = Session::default(); + let e = s.execute(spec.id, &json!({"bogus_param": 1})).unwrap_err().to_string(); + // A command that cannot run yet says so first; otherwise the key is named. + assert!(e.contains("bogus_param") || s.availability(spec).is_err(), "{}: {e}", spec.id); + } + let mut s = Session::default(); + crate::cmd::solve::tests::cantilever(&mut s); + s.execute("solve.run", &json!({})).unwrap(); + let e = s.execute("results.contour", &json!({"field": "displacement", "deformed": true})).unwrap_err().to_string(); + assert!(e.contains("`deformed`") && e.contains("field"), "{e}"); + } } diff --git a/crates/model/src/lib.rs b/crates/model/src/lib.rs index 1d99b36..7174338 100644 --- a/crates/model/src/lib.rs +++ b/crates/model/src/lib.rs @@ -294,10 +294,30 @@ pub fn check_name(what: &str, name: &str) -> Result<()> { Ok(()) } else { let shown: String = name.chars().take(MAX_NAME_LEN).collect(); - Err(Error::Invalid(format!("{what} name `{shown}` must be 1 to {MAX_NAME_LEN} characters: a letter, then letters, digits, `_` or `-`"))) + let rule = format!("{what} name `{shown}` must be 1 to {MAX_NAME_LEN} characters: a letter, then letters, digits, `_` or `-`"); + Err(Error::Invalid(match suggest_name(name) { + Some(fixed) => format!("{rule} (try `{fixed}`)"), + None => rule, + })) } } +/// The valid name closest to `name`: spaces and other characters become `_`, a leading +/// non-letter gets an `N` before it, and it is cut to [`MAX_NAME_LEN`]. `None` when nothing of it +/// is usable (empty, or no letter or digit at all). +pub fn suggest_name(name: &str) -> Option { + let trimmed = name.trim(); + if !trimmed.chars().any(|c| c.is_ascii_alphanumeric()) { + return None; + } + let mut fixed: String = trimmed.chars().map(|c| if c.is_ascii_alphanumeric() || c == '-' { c } else { '_' }).collect(); + if !fixed.starts_with(|c: char| c.is_ascii_alphabetic()) { + fixed.insert(0, 'N'); + } + fixed.truncate(MAX_NAME_LEN); + Some(fixed) +} + /// Check that a value is a finite number, naming it in the error. pub(crate) fn finite(what: &str, v: f64) -> Result { if v.is_finite() { Ok(v) } else { Err(Error::Invalid(format!("{what} must be a finite number, got {v}"))) } @@ -412,3 +432,17 @@ impl Model { ] } } + +#[cfg(test)] +mod name_tests { + use super::*; + + #[test] + fn a_bad_name_suggests_a_fixed_one() { + let e = check_name("load", "Hot air").unwrap_err().to_string(); + assert!(e.contains("try `Hot_air`"), "{e}"); + assert_eq!(suggest_name("1st load"), Some("N1st_load".into())); + assert_eq!(suggest_name(" "), None); + assert!(check_name("load", &suggest_name(&"x ".repeat(80)).unwrap()).is_ok()); + } +}