mirror of
https://github.com/bherbruck/solidcraft.git
synced 2026-11-03 02:25:55 +00:00
* Every new command comes with input and output schemas A test fails for a command without a contract or an output type, unless it is a planned! placeholder or listed in schema/unconverted.txt, which holds the 400 commands that predate the rule and only shrinks: the test also fails when a listed command has its schemas, when the list names a command that doesn't exist, or when it isn't sorted. `checked!` now takes only the command's id and checks the contract that id publishes (`run_checked_as`), so the contract enforced is always the one described. Another test runs every command that has a contract in strict with an unknown key, so a contract is never published without `checked!`, under the command's own id, enforcing it. A new book page, Command schemas, explains how to write a contract and a reply type, the tests to add, the shared field types and how to add one, with a checklist; the failure messages point to it. "Adding a command", CONTRIBUTING.md and AGENTS.md now include the schemas, and the MCP tool description and docs/mcp.md no longer list the converted commands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List the commands added on main since: view.layout, view.history_layout, view.normal_to Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Command schemas guide: what participants and targets do, checking the evaluation, warning-free replies - `participants` changes the result for some features and nothing for others; where it and another key override each other, or where it has no effect, say so in that key's description or in the summary. - The example primitive's `targets` default to every body in the feature's component, not the bodies it meets; they're ignored when `participants` lists bodies. - Check what each value does where the feature is evaluated, not only how the handler reads it. - A reply test runs every form in Warn and checks for no `param_warnings` (Strict for a reply that isn't an object). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List the 43 commands added on main since: frame, component, render and others Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * List the 11 commands added on main since: browser folding, standard views, parameter sweep, path flange browser.collapse_all, browser.expand_all, inspect.parameter_sweep, sheet.path_flange and the seven view.* standard views (back, bottom, front, isometric, left, right, top) landed on main without schemas; the gate flags them, as intended. 457 commands listed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Schema rule warns instead of failing for now A command without input and output schemas prints a warning listing it (test output and a CI annotation). SCHEMA_RULE_STRICT in coverage_tests.rs (or SOLIDCRAFT_SCHEMA_RULE_STRICT=1) makes it fail again. Docs say "should, and will become required". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: bherbruck <brennen.herbruck@gmail.com>
This commit is contained in:
@@ -82,7 +82,10 @@ People trust SolidCraft with their designs; a crash loses their work. **This out
|
||||
- **Everything is a command.** User-visible behaviour = a command in `crates/engine/src/cmd/*`
|
||||
(`CommandSpec`: id = our own dotted, lower-case id `<area>.<thing>[.<variant>]` such as
|
||||
`solid.extrude` or `sketch.rectangle.two_point`, never another product's command id; label;
|
||||
toolbar tab and panel; icon; shortcut; params doc; `enabled`; JSON `run`) + tests. The toolbar,
|
||||
toolbar tab and panel; icon; shortcut; params doc; `enabled`; JSON `run`) + input and output
|
||||
schemas (a contract and a reply type in `crates/engine/src/schema/`, registered with
|
||||
`checked!`, `.contract` and `.output`; a command should have them, and a test warns
|
||||
without them (it will become required); `schema/unconverted.txt` only shrinks; how: docs/book/src/dev/schemas.md) + tests. The toolbar,
|
||||
dialogs, sketch tools, palette, scripts, CLI and control channel all reach the same commands.
|
||||
- **Programmatic calls never open dialogs.** `engine.execute` runs the JSON form; only toolbar
|
||||
clicks (`ui.start`) start interactive tools and dialogs, which then run commands.
|
||||
|
||||
+6
-1
@@ -130,7 +130,9 @@ The [architecture page](docs/architecture.md) has the layer table. The short ver
|
||||
- Every call into truck goes through `solidcraft_kernel::guard`.
|
||||
- Every crash fix comes with a regression test.
|
||||
5. **Everything is a command, with a test.** A new feature means a `CommandSpec` in
|
||||
`crates/engine/src/cmd/`, engine tests that measure the result (volume, area,
|
||||
`crates/engine/src/cmd/` with input and output schemas (see
|
||||
[Adding a command](docs/book/src/dev/commands.md#adding-a-command); a test warns about a
|
||||
command without them, and will fail it once required), engine tests that measure the result (volume, area,
|
||||
face/edge/vertex counts against a value you can derive), and for UI work a scenario (below).
|
||||
6. **One set of camera controls.** Every page or canvas that shows a view (the modelling
|
||||
viewport, the Render preview, and any new page such as a flat pattern or drawing) calls
|
||||
@@ -274,6 +276,9 @@ and a fresh CI run:
|
||||
and the Lists workflow) fails on an unsorted catalog or an id that appears twice.
|
||||
- **`mod` lines**: one per line, in the sorted group they belong to (`cargo fmt` keeps each group
|
||||
sorted). Add a new module into the existing group, not as a new group at the end.
|
||||
- **Commands without schemas** (`crates/engine/src/schema/unconverted.txt`): sorted, one id per
|
||||
line, and it only shrinks. Converting a command removes its line; a new command is never added
|
||||
to it.
|
||||
- **Languages** (`crates/ui-egui/src/i18n.rs`): one per line, English first, then by id (a test
|
||||
checks it).
|
||||
- **Generated files**: `THIRD-PARTY-LICENSES.txt` and `docs/features.md` are rewritten on `main`
|
||||
|
||||
@@ -19,7 +19,7 @@ const BOX_CONTRACT: TypedContract<BoxParams> = TypedContract::new();
|
||||
const EXTRUDE_CONTRACT: TypedContract<ExtrudeParams> = TypedContract::new();
|
||||
|
||||
pub static COMMANDS: &[CommandSpec] = &[
|
||||
CommandSpec::new("solid.extrude", "Extrude", crate::checked!("solid.extrude", EXTRUDE_CONTRACT, extrude))
|
||||
CommandSpec::new("solid.extrude", "Extrude", crate::checked!("solid.extrude", extrude))
|
||||
.at("SOLID", "CREATE")
|
||||
.icon("extrude")
|
||||
.key("E")
|
||||
@@ -35,7 +35,7 @@ pub static COMMANDS: &[CommandSpec] = &[
|
||||
.icon("sweep")
|
||||
.params("sketch: profile sketch; profiles?; path_sketch: sketch; path: [curve ids in order]; orientation?: perpendicular (default: square to the path, turning with it) | parallel (as drawn: the profile moves without turning; the path must keep crossing its plane) | [x,y,z] (square to the path, its turn about the path fixed by this direction); twist?: angle expr (the profile turns this much about the path over its length; within 4 turns); scale?: expr (the profile's scale at the end, linear along the path; 0.01…100); rails?: [{sketch?: sketch (default path_sketch), curves: [curve ids in order]}] (guide rails, one or two, each meeting every plane square to the path once along it, from the end nearest the path's start: one rail turns the profile so its point where the rail starts stays on the rail and scales it with the rail's distance from the path; two rails carry it between them, its points where they start staying on both; rails take the default orientation, no twist and no scale, and a path without sharp corners); rail_fit?: scale (default: the profile grows and shrinks evenly) | stretch (only across to the rail, between the rails); operation?, targets?, name?, body_name?"),
|
||||
CommandSpec::new("solid.loft", "Loft", loft).at("SOLID", "CREATE").icon("loft").params("sections: [{sketch, profiles?} | {sketch, point: id} (an apex)] (2…50, in order; matching hole counts, no holes at an apex); rails?: [{sketch, curves: [ids in order]}] (one or two guide rails from the first section to the last, through every section's outline: the loft follows them between the sections; not with sections that have holes); rail_fit?: scale|stretch (default scale: with one rail the sections grow evenly with the rail's distance from their middle, with two with the distance between the rails; stretch: only that way); operation?, targets?, name?, body_name?"),
|
||||
CommandSpec::new("solid.box", "Box", crate::checked!("solid.box", BOX_CONTRACT, prim_box))
|
||||
CommandSpec::new("solid.box", "Box", crate::checked!("solid.box", prim_box))
|
||||
.at("SOLID", "CREATE")
|
||||
.icon("box")
|
||||
.contract(&BOX_CONTRACT)
|
||||
|
||||
@@ -47,7 +47,7 @@ pub static COMMANDS: &[CommandSpec] = &[
|
||||
.doc_only()
|
||||
.params("→ built-in and design-owned materials (user: true, look, optional physical_card/physical_validation/density_comparison). Read-only; candidate engineering records do not certify sources or supply a solver"),
|
||||
CommandSpec::new("engine.commands", "List Commands", commands).icon("list").noundo(),
|
||||
CommandSpec::new("engine.describe", "Describe Command", crate::checked!("engine.describe", DESCRIBE_CONTRACT, describe))
|
||||
CommandSpec::new("engine.describe", "Describe Command", crate::checked!("engine.describe", describe))
|
||||
.icon("list")
|
||||
.noundo()
|
||||
.doc_only()
|
||||
|
||||
@@ -115,7 +115,7 @@ impl<P: DeserializeOwned + JsonSchema + 'static> Contract for TypedContract<P> {
|
||||
}
|
||||
|
||||
/// The most keys of one call that are looked at, and the longest key given a suggestion.
|
||||
const MAX_KEYS: usize = 64;
|
||||
pub(crate) const MAX_KEYS: usize = 64;
|
||||
const MAX_KEY_LEN: usize = 64;
|
||||
const MAX_MESSAGE_LEN: usize = 300;
|
||||
const MAX_WHOLE_LEN: usize = 4096;
|
||||
@@ -260,6 +260,20 @@ pub(crate) fn parse_issue(e: &serde_path_to_error::Error<serde_json::Error>) ->
|
||||
Issue { path, message: truncate(&message, MAX_MESSAGE_LEN), suggestion: None }
|
||||
}
|
||||
|
||||
/// Run `handler` as the command `id`, checking the call against the contract `id` publishes
|
||||
/// (its [`CommandSpec::contract`](crate::cmd::CommandSpec::contract)), so the contract enforced is
|
||||
/// always the one described. With the level off and no audit recorder, this is exactly
|
||||
/// `handler(s, p)`, without looking anything up; a command without a contract just runs.
|
||||
pub fn run_checked_as(s: &mut Session, p: &Value, id: &str, handler: Run) -> Result<Value> {
|
||||
if s.param_check == ParamCheck::Off && !record::enabled() {
|
||||
return handler(s, p);
|
||||
}
|
||||
match crate::cmd::find_command(id).and_then(|c| c.contract) {
|
||||
Some(contract) => run_checked(s, p, id, contract, handler),
|
||||
None => handler(s, p),
|
||||
}
|
||||
}
|
||||
|
||||
/// Run `handler` as the command `id`, checking the call against `contract` as the session's
|
||||
/// [`ParamCheck`] level asks (see the module docs). With the level off and no audit recorder,
|
||||
/// this is exactly `handler(s, p)`.
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
//! New commands come with schemas: every command has a contract (its input schema) and an
|
||||
//! output schema, apart from `planned!` placeholders and the commands listed in
|
||||
//! `unconverted.txt`. That list holds the commands that predate the rule; it only shrinks.
|
||||
|
||||
use std::collections::BTreeSet;
|
||||
|
||||
use serde_json::json;
|
||||
|
||||
use crate::Session;
|
||||
use crate::cmd::command_specs;
|
||||
use crate::schema::ParamCheck;
|
||||
use crate::schema::check::MAX_KEYS;
|
||||
|
||||
/// Whether a command without schemas fails the test. Off for now: the test warns instead. The
|
||||
/// owner will flip this to `true` when the PR backlog is down. `SOLIDCRAFT_SCHEMA_RULE_STRICT=1`
|
||||
/// turns it on for one run.
|
||||
const SCHEMA_RULE_STRICT: bool = false;
|
||||
|
||||
const UNCONVERTED: &str = include_str!("unconverted.txt");
|
||||
|
||||
/// The ids in `unconverted.txt`, in file order (`#` lines are comments).
|
||||
fn unconverted() -> Vec<&'static str> {
|
||||
UNCONVERTED.lines().map(str::trim).filter(|l| !l.is_empty() && !l.starts_with('#')).collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_command_has_input_and_output_schemas() {
|
||||
let listed: BTreeSet<&str> = unconverted().into_iter().collect();
|
||||
let mut problems = Vec::new();
|
||||
let mut missing = Vec::new();
|
||||
for c in command_specs() {
|
||||
let has = (c.contract.is_some(), c.output.is_some());
|
||||
let listed = listed.contains(c.id);
|
||||
if c.planned.is_some() {
|
||||
if listed {
|
||||
problems.push(format!("`{}` is a planned! placeholder, which needs no line in unconverted.txt", c.id));
|
||||
}
|
||||
continue;
|
||||
}
|
||||
match (has, listed) {
|
||||
((true, true), true) => problems.push(format!("`{}` has its schemas now: remove its line from unconverted.txt", c.id)),
|
||||
((true, true), false) | (_, true) => {}
|
||||
((input, output), false) => missing.push(format!(
|
||||
"`{}` has no {} schema. Every new command needs a contract (`.contract(…)`, run through `checked!`) \
|
||||
and an output type (`.output(…)`): see docs/book/src/dev/schemas.md",
|
||||
c.id,
|
||||
match (input, output) {
|
||||
(true, _) => "output",
|
||||
(false, true) => "input",
|
||||
(false, false) => "input or output",
|
||||
}
|
||||
)),
|
||||
}
|
||||
}
|
||||
assert!(problems.is_empty(), "{}", problems.join("\n"));
|
||||
if !missing.is_empty() {
|
||||
let strict = SCHEMA_RULE_STRICT || std::env::var_os("SOLIDCRAFT_SCHEMA_RULE_STRICT").is_some_and(|v| v != "0");
|
||||
let report = format!(
|
||||
"{} command(s) lack an input or output schema. This is a warning for now; it will become required:\n{}",
|
||||
missing.len(),
|
||||
missing.join("\n")
|
||||
);
|
||||
assert!(!strict, "{report}");
|
||||
eprintln!("warning: {report}");
|
||||
// A GitHub Actions annotation, so the warning shows on the CI run page too.
|
||||
println!("::warning title=Commands without schemas::{}", missing.len());
|
||||
}
|
||||
}
|
||||
|
||||
/// A contract that is published but not checked would describe calls nobody refuses: every
|
||||
/// command with a contract runs through `checked!` with its own id, which checks the contract
|
||||
/// that id publishes. Calls `run` itself (not `Session::execute`, which first asks `enabled`)
|
||||
/// in strict with an unknown key, which must be refused under the command's own id before the
|
||||
/// handler sees it.
|
||||
#[test]
|
||||
fn every_contract_is_checked_in_front_of_its_handler() {
|
||||
const PROBE: &str = "not_a_key_of_any_command";
|
||||
for c in command_specs() {
|
||||
let Some(contract) = c.contract else { continue };
|
||||
// The check reads at most `MAX_KEYS` keys of a call, so a contract can't take more.
|
||||
let keys = contract.allowed_keys().len();
|
||||
assert!(keys <= MAX_KEYS, "`{}` takes {keys} keys; a call may have at most {MAX_KEYS}, so no call could use them all", c.id);
|
||||
let mut s = Session { param_check: ParamCheck::Strict, ..Session::default() };
|
||||
let run = c.run;
|
||||
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| run(&mut s, &json!({PROBE: 1}))));
|
||||
let fix = format!("give its CommandSpec `crate::checked!(\"{}\", <its handler>)`", c.id);
|
||||
let error = match result {
|
||||
Ok(Err(e)) => e.to_string(),
|
||||
Ok(Ok(v)) => panic!("`{}` ran with an unknown key in strict ({v}): {fix}", c.id),
|
||||
Err(_) => panic!("`{}` panicked on an unknown key in strict: {fix}", c.id),
|
||||
};
|
||||
let expected = format!("invalid parameters for `{}`: unknown key `{PROBE}`", c.id);
|
||||
assert!(error.starts_with(&expected), "`{}` should refuse with \"{expected}\", not \"{error}\": {fix}", c.id);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_unconverted_list_is_sorted_and_names_real_commands() {
|
||||
let ids = unconverted();
|
||||
for pair in ids.windows(2) {
|
||||
assert!(pair[0] < pair[1], "unconverted.txt must be sorted, each id once: `{}` comes before `{}`", pair[0], pair[1]);
|
||||
}
|
||||
let registered: BTreeSet<&str> = command_specs().iter().map(|c| c.id).collect();
|
||||
let stale: Vec<&str> = ids.into_iter().filter(|id| !registered.contains(id)).collect();
|
||||
assert!(stale.is_empty(), "unconverted.txt names commands that don't exist (remove their lines): {stale:?}");
|
||||
}
|
||||
@@ -36,12 +36,20 @@
|
||||
//! `strict_keeps_todays_errors`. Then run the whole test suite with the audit recorder (see
|
||||
//! `record.rs`): a call that succeeds today but breaks the contract is either a form the
|
||||
//! contract is missing or a caller's mistake.
|
||||
//!
|
||||
//! New commands come with both schemas: `coverage_tests.rs` warns about a command without a
|
||||
//! contract or an output type (it will fail once `SCHEMA_RULE_STRICT` flips), unless it is a `planned!` placeholder or listed in
|
||||
//! `unconverted.txt`, which holds the commands that predate the rule. That list only shrinks:
|
||||
//! converting a command removes its line, and nothing is added to it. How to write a contract,
|
||||
//! with the shared types and a checklist: docs/book/src/dev/schemas.md.
|
||||
|
||||
mod check;
|
||||
#[cfg_attr(target_arch = "wasm32", path = "record_wasm.rs")]
|
||||
mod record;
|
||||
mod types;
|
||||
|
||||
#[cfg(test)]
|
||||
mod coverage_tests;
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
@@ -49,16 +57,17 @@ mod tests;
|
||||
pub mod features;
|
||||
pub mod inspect;
|
||||
|
||||
pub use check::{Contract, Issue, ParamCheck, TypedContract, input_schema_for, output_schema_for, run_checked};
|
||||
pub use check::{Contract, Issue, ParamCheck, TypedContract, input_schema_for, output_schema_for, run_checked, run_checked_as};
|
||||
pub use types::*;
|
||||
|
||||
/// A command's `run` that checks the call against `$contract` before running `$handler`
|
||||
/// (see [`run_checked`]). Expands to a non-capturing closure, which coerces to the registry's
|
||||
/// `Run` function pointer.
|
||||
/// A command's `run` that checks the call against the contract command `$id` publishes
|
||||
/// (`.contract(…)` on its `CommandSpec`) before running `$handler` (see [`run_checked_as`]), so
|
||||
/// the contract enforced is always the one described. Expands to a non-capturing closure, which
|
||||
/// coerces to the registry's `Run` function pointer.
|
||||
#[macro_export]
|
||||
#[doc(hidden)]
|
||||
macro_rules! checked {
|
||||
($id:literal, $contract:path, $handler:path) => {
|
||||
|s: &mut $crate::Session, p: &serde_json::Value| $crate::schema::run_checked(s, p, $id, &$contract, $handler)
|
||||
($id:literal, $handler:path) => {
|
||||
|s: &mut $crate::Session, p: &serde_json::Value| $crate::schema::run_checked_as(s, p, $id, $handler)
|
||||
};
|
||||
}
|
||||
|
||||
@@ -0,0 +1,461 @@
|
||||
# Commands that predate the rule that every command has input and output schemas
|
||||
# (coverage_tests.rs). This list only shrinks: when you give a command its contract and output
|
||||
# type, remove its line. Never add one: a new command comes with both schemas. Sorted, one id
|
||||
# per line.
|
||||
appearance.assign
|
||||
appearance.library
|
||||
appearance.list
|
||||
assembly.bom
|
||||
body.rename
|
||||
browser.collapse_all
|
||||
browser.expand_all
|
||||
browser.group
|
||||
browser.move
|
||||
browser.order
|
||||
browser.rename_group
|
||||
browser.ungroup
|
||||
browser.visibility
|
||||
canvas.calibrate
|
||||
canvas.decal
|
||||
canvas.delete
|
||||
canvas.edit
|
||||
canvas.insert
|
||||
component.activate
|
||||
component.break_link
|
||||
component.capture_position
|
||||
component.copies
|
||||
component.create
|
||||
component.delete
|
||||
component.extract
|
||||
component.from_bodies
|
||||
component.insert_design
|
||||
component.links
|
||||
component.list
|
||||
component.metadata
|
||||
component.metadata.set
|
||||
component.move_bodies
|
||||
component.move_sketches
|
||||
component.paste_new
|
||||
component.properties
|
||||
component.relink
|
||||
component.rename
|
||||
component.replace_with_link
|
||||
component.revert_position
|
||||
component.unlock
|
||||
component.update_links
|
||||
config.activate
|
||||
config.column
|
||||
config.configure
|
||||
config.preview
|
||||
config.row
|
||||
config.rule
|
||||
config.rules
|
||||
config.table
|
||||
construct.axis.edge
|
||||
construct.axis.normal_to_face
|
||||
construct.axis.through_cylinder
|
||||
construct.axis.two_planes
|
||||
construct.axis.two_points
|
||||
construct.plane.along_path
|
||||
construct.plane.at_angle
|
||||
construct.plane.midplane
|
||||
construct.plane.offset
|
||||
construct.plane.perpendicular
|
||||
construct.plane.tangent
|
||||
construct.plane.three_points
|
||||
construct.plane.two_edges
|
||||
construct.point.along_path
|
||||
construct.point.center
|
||||
construct.point.edge_plane
|
||||
construct.point.three_planes
|
||||
construct.point.two_edges
|
||||
construct.point.vertex
|
||||
contact.check
|
||||
contact.create
|
||||
contact.disable_all
|
||||
contact.edit
|
||||
contact.enable_all
|
||||
contact.enable_sets
|
||||
contact.list
|
||||
diagnostics.crash_report
|
||||
diagnostics.crash_reports
|
||||
diagnostics.crash_reports.clear
|
||||
diagnostics.crash_test
|
||||
doc.open
|
||||
doc.recover
|
||||
doc.recovery_discard
|
||||
doc.recovery_list
|
||||
document.inspect
|
||||
document.metadata
|
||||
document.metadata.remove
|
||||
document.metadata.set
|
||||
document.units
|
||||
edit.checkpoint
|
||||
edit.redo
|
||||
edit.restore_checkpoint
|
||||
edit.undo
|
||||
engine.commands
|
||||
explode.create
|
||||
explode.delete
|
||||
explode.list
|
||||
explode.show
|
||||
expr.evaluate
|
||||
face.delete
|
||||
file.export
|
||||
file.export_view_dxf
|
||||
file.insert_mesh
|
||||
file.insert_step
|
||||
file.new
|
||||
file.save
|
||||
file.save_as
|
||||
file.save_mesh
|
||||
file.slicers
|
||||
frame.member
|
||||
frame.profile.insert
|
||||
frame.profile.list
|
||||
frame.profile.properties
|
||||
inspect.accessibility
|
||||
inspect.center_of_mass
|
||||
inspect.clearance
|
||||
inspect.compare
|
||||
inspect.continuity
|
||||
inspect.curvature_comb
|
||||
inspect.curvature_map
|
||||
inspect.draft
|
||||
inspect.environment_map
|
||||
inspect.interference
|
||||
inspect.isocurve
|
||||
inspect.measure
|
||||
inspect.minimum_radius
|
||||
inspect.parameter_sweep
|
||||
inspect.section
|
||||
inspect.silhouette
|
||||
inspect.zebra
|
||||
joint.as_built
|
||||
joint.cam
|
||||
joint.create
|
||||
joint.delete
|
||||
joint.drive
|
||||
joint.edit
|
||||
joint.gear
|
||||
joint.inspect
|
||||
joint.limits
|
||||
joint.list
|
||||
joint.motion_link
|
||||
joint.origin
|
||||
joint.rack_pinion
|
||||
joint.reach
|
||||
joint.rigid_group
|
||||
joint.screw
|
||||
joint.solve
|
||||
joint.sweep
|
||||
library.apply
|
||||
library.export
|
||||
library.inspect
|
||||
material.assign
|
||||
material.create
|
||||
material.delete
|
||||
material.edit
|
||||
material.inspect
|
||||
material.list
|
||||
material.properties.directional
|
||||
material.properties.validate
|
||||
mesh.convert
|
||||
model.edges
|
||||
model.faces
|
||||
model.threads
|
||||
motion.delete
|
||||
motion.export
|
||||
motion.export_animation
|
||||
motion.interference
|
||||
motion.list
|
||||
motion.play
|
||||
motion.plot
|
||||
motion.study
|
||||
motion.trace
|
||||
motion.trace.edit
|
||||
motion.trace.keep
|
||||
occurrence.copy
|
||||
occurrence.drag
|
||||
occurrence.ground
|
||||
occurrence.move
|
||||
occurrence.properties
|
||||
parameters.add
|
||||
parameters.change
|
||||
parameters.comment
|
||||
parameters.complete
|
||||
parameters.delete
|
||||
parameters.export
|
||||
parameters.favorite
|
||||
parameters.graph
|
||||
parameters.import
|
||||
parameters.list
|
||||
parameters.move
|
||||
parameters.rename
|
||||
parameters.users
|
||||
parts.fastener
|
||||
parts.insert
|
||||
parts.library
|
||||
parts.resize
|
||||
plastic.assign_rule
|
||||
plastic.boss
|
||||
plastic.lip
|
||||
plastic.manage_rules
|
||||
plastic.rest
|
||||
plastic.rules
|
||||
plastic.snap_fit
|
||||
render.aging.describe
|
||||
render.backends
|
||||
render.camera.fit
|
||||
render.camera.get
|
||||
render.camera.set
|
||||
render.cancel
|
||||
render.effects.describe
|
||||
render.export
|
||||
render.hdr.inspect
|
||||
render.library.appearance.apply
|
||||
render.library.describe
|
||||
render.library.environment.apply
|
||||
render.library.list
|
||||
render.library.physical.prepare
|
||||
render.library.preview
|
||||
render.library.studio.apply
|
||||
render.library.studio.prepare
|
||||
render.library.surface.bind
|
||||
render.library.surface.prepare
|
||||
render.library.surface.unbind
|
||||
render.library.targets
|
||||
render.optics.bind
|
||||
render.optics.unbind
|
||||
render.preflight
|
||||
render.preset.apply
|
||||
render.preset.resolve
|
||||
render.quality.prepare
|
||||
render.result.pin
|
||||
render.result.regrade
|
||||
render.scene
|
||||
render.settings.apply
|
||||
render.settings.capture_hdr
|
||||
render.settings.get
|
||||
render.settings.set
|
||||
render.shot.duplicate
|
||||
render.shot.remove
|
||||
render.shot.rename
|
||||
render.stage.backdrop
|
||||
render.stage.clear
|
||||
render.stage.ground
|
||||
render.start
|
||||
render.status
|
||||
select.clear
|
||||
select.set
|
||||
selection.delete
|
||||
sheet.convert
|
||||
sheet.export_bend_table
|
||||
sheet.export_dxf
|
||||
sheet.flange
|
||||
sheet.flat_pattern
|
||||
sheet.fold
|
||||
sheet.hem
|
||||
sheet.manage_rules
|
||||
sheet.path_flange
|
||||
sheet.perforate
|
||||
sheet.refold
|
||||
sheet.rules
|
||||
sheet.unfold
|
||||
sketch.arc.center_point
|
||||
sketch.arc.tangent
|
||||
sketch.arc.three_point
|
||||
sketch.auto_constrain
|
||||
sketch.auto_constrain_from_datum
|
||||
sketch.auto_project
|
||||
sketch.blend_curve
|
||||
sketch.break
|
||||
sketch.break_link
|
||||
sketch.cancel
|
||||
sketch.centerline
|
||||
sketch.chain
|
||||
sketch.chamfer.distance_angle
|
||||
sketch.chamfer.equal_distance
|
||||
sketch.chamfer.two_distance
|
||||
sketch.circle.center
|
||||
sketch.circle.three_point
|
||||
sketch.circle.three_tangent
|
||||
sketch.circle.two_point
|
||||
sketch.circle.two_tangent
|
||||
sketch.conic
|
||||
sketch.constrainer
|
||||
sketch.constraint.coincident
|
||||
sketch.constraint.collinear
|
||||
sketch.constraint.concentric
|
||||
sketch.constraint.curvature
|
||||
sketch.constraint.equal
|
||||
sketch.constraint.fix
|
||||
sketch.constraint.horizontal_vertical
|
||||
sketch.constraint.midpoint
|
||||
sketch.constraint.parallel
|
||||
sketch.constraint.perpendicular
|
||||
sketch.constraint.polygon
|
||||
sketch.constraint.symmetry
|
||||
sketch.constraint.tangent
|
||||
sketch.construction
|
||||
sketch.copy
|
||||
sketch.create
|
||||
sketch.cut
|
||||
sketch.delete
|
||||
sketch.dimension
|
||||
sketch.dimension_text
|
||||
sketch.drag
|
||||
sketch.drag_end
|
||||
sketch.draw.dimensioned
|
||||
sketch.edit
|
||||
sketch.edit_text
|
||||
sketch.ellipse
|
||||
sketch.export_dxf
|
||||
sketch.extend
|
||||
sketch.fillet
|
||||
sketch.finish
|
||||
sketch.finish_auto_constrain
|
||||
sketch.fit_curves_to_section
|
||||
sketch.glyphs
|
||||
sketch.include_3d
|
||||
sketch.infinite
|
||||
sketch.insert_dxf
|
||||
sketch.insert_svg
|
||||
sketch.inspect
|
||||
sketch.intersect
|
||||
sketch.intersection_curve
|
||||
sketch.isoparametric_curve
|
||||
sketch.line
|
||||
sketch.line.midpoint
|
||||
sketch.line3d
|
||||
sketch.loose_ends
|
||||
sketch.loose_ends.tie
|
||||
sketch.mirror
|
||||
sketch.move
|
||||
sketch.move3d
|
||||
sketch.move_point
|
||||
sketch.offset
|
||||
sketch.offset.break
|
||||
sketch.options
|
||||
sketch.paste
|
||||
sketch.pattern.circular
|
||||
sketch.pattern.rectangular
|
||||
sketch.point
|
||||
sketch.point3d
|
||||
sketch.polygon.circumscribed
|
||||
sketch.polygon.edge
|
||||
sketch.polygon.inscribed
|
||||
sketch.project
|
||||
sketch.project_to_surface
|
||||
sketch.rectangle.center
|
||||
sketch.rectangle.rounded
|
||||
sketch.rectangle.three_point
|
||||
sketch.rectangle.two_point
|
||||
sketch.redefine
|
||||
sketch.scale
|
||||
sketch.slot.arc_center
|
||||
sketch.slot.arc_three_point
|
||||
sketch.slot.center_point
|
||||
sketch.slot.center_to_center
|
||||
sketch.slot.overall
|
||||
sketch.snap
|
||||
sketch.solve
|
||||
sketch.spline.control_point
|
||||
sketch.spline.control_point_5
|
||||
sketch.spline.fit_point
|
||||
sketch.spline3d
|
||||
sketch.spun_profile
|
||||
sketch.text
|
||||
sketch.toggle_driven
|
||||
sketch.trim
|
||||
sketch.trim.path
|
||||
solid.align
|
||||
solid.boundary_fill
|
||||
solid.bounding_solid
|
||||
solid.chamfer
|
||||
solid.coil
|
||||
solid.combine
|
||||
solid.cylinder
|
||||
solid.draft
|
||||
solid.emboss
|
||||
solid.fillet
|
||||
solid.hole
|
||||
solid.loft
|
||||
solid.mirror
|
||||
solid.move
|
||||
solid.offset_face
|
||||
solid.pattern.circular
|
||||
solid.pattern.path
|
||||
solid.pattern.rectangular
|
||||
solid.pipe
|
||||
solid.press_pull
|
||||
solid.remove
|
||||
solid.replace_face
|
||||
solid.revolve
|
||||
solid.rib
|
||||
solid.scale
|
||||
solid.shell
|
||||
solid.sphere
|
||||
solid.split_body
|
||||
solid.split_face
|
||||
solid.sweep
|
||||
solid.thread
|
||||
solid.torus
|
||||
solid.web
|
||||
surface.blend
|
||||
surface.boundary
|
||||
surface.extend
|
||||
surface.extrude
|
||||
surface.fair
|
||||
surface.loft
|
||||
surface.match
|
||||
surface.offset
|
||||
surface.patch
|
||||
surface.reverse_normal
|
||||
surface.revolve
|
||||
surface.ruled
|
||||
surface.stitch
|
||||
surface.sweep
|
||||
surface.thicken
|
||||
surface.trim
|
||||
surface.unstitch
|
||||
surface.untrim
|
||||
timeline.add
|
||||
timeline.compute_all
|
||||
timeline.copy
|
||||
timeline.delete
|
||||
timeline.dependents
|
||||
timeline.edit
|
||||
timeline.export_script
|
||||
timeline.group
|
||||
timeline.inspect
|
||||
timeline.move_many
|
||||
timeline.move_to_group
|
||||
timeline.paste
|
||||
timeline.redefine
|
||||
timeline.rename
|
||||
timeline.rename_group
|
||||
timeline.reorder
|
||||
timeline.reorder_many
|
||||
timeline.roll_to
|
||||
timeline.rollback
|
||||
timeline.suppress
|
||||
timeline.ungroup
|
||||
view.ambient_occlusion
|
||||
view.apply
|
||||
view.back
|
||||
view.bottom
|
||||
view.delete
|
||||
view.front
|
||||
view.graphics_preset
|
||||
view.history_layout
|
||||
view.isometric
|
||||
view.layout
|
||||
view.left
|
||||
view.list
|
||||
view.navigate
|
||||
view.normal_to
|
||||
view.rename
|
||||
view.right
|
||||
view.save
|
||||
view.top
|
||||
@@ -75,7 +75,7 @@ pub fn tool_definitions() -> Vec<Value> {
|
||||
tool(
|
||||
"describe_command",
|
||||
"Describe a command",
|
||||
"One command's parameter docs and, for commands with a typed contract (solid.box, solid.extrude and engine.describe so far), the \
|
||||
"One command's parameter docs and, for commands with a typed contract (every new command, and more of the older ones as they are converted), the \
|
||||
JSON Schemas (2020-12) of its parameters (`input`) and of a successful result (`output`); null for the others.",
|
||||
obj(json!({"id": string("Command id, e.g. \"solid.extrude\"")}), &["id"]),
|
||||
Effect::ReadOnly,
|
||||
|
||||
@@ -27,6 +27,7 @@
|
||||
- [Extensible library proposal](dev/libraries.md)
|
||||
- [Local scalar libraries](dev/local-libraries.md)
|
||||
- [The command engine](dev/commands.md)
|
||||
- [Command schemas](dev/schemas.md)
|
||||
- [The kernel boundary and truck](dev/kernel.md)
|
||||
- [Persistent naming](dev/naming.md)
|
||||
- [Test harnesses](dev/testing.md)
|
||||
|
||||
@@ -31,9 +31,21 @@ ending in the same word (`file.open`: did you mean `doc.open`?).
|
||||
## Adding a command
|
||||
|
||||
1. Add a `CommandSpec` to the right file in `crates/engine/src/cmd/` and implement `run`.
|
||||
2. Write engine tests that measure the result: volume, area, and face/edge/vertex counts
|
||||
2. Give it input and output schemas: a parameters type (its contract) and a reply type in the
|
||||
module for its command file under `crates/engine/src/schema/` (`schema/features.rs` for
|
||||
`cmd/features.rs`, and so on; create it for the file's first contract), built from the shared
|
||||
field types in `schema/types.rs`. Register them with `checked!(…)`, `.contract(…)` and
|
||||
`.output(…)`, as `solid.extrude` does, and add the command to that module's
|
||||
`schemas_and_the_key_check_agree` test, with error-message cases and a reply test. The
|
||||
parameters type documents each parameter (keep the `.params(…)` prose too, for now), and
|
||||
`describe_command` serves both schemas to agents and scripts. A test
|
||||
(`every_command_has_input_and_output_schemas`) warns for a command without them, and will
|
||||
fail it once the rule becomes required; the commands
|
||||
that predate the rule are listed in `schema/unconverted.txt`, which only shrinks.
|
||||
[Command schemas](schemas.md) walks through it, with the shared types and a checklist.
|
||||
3. Write engine tests that measure the result: volume, area, and face/edge/vertex counts
|
||||
against values you can derive by hand.
|
||||
3. The hostile-input test (`hostile_params_never_panic` in `crates/engine/src/tests.rs`) runs
|
||||
4. The hostile-input test (`hostile_params_never_panic` in `crates/engine/src/tests.rs`) runs
|
||||
your command automatically with every documented parameter set to junk. It must return
|
||||
errors, never panic.
|
||||
4. For UI work, add a scenario (see [Test harnesses](testing.md)).
|
||||
5. For UI work, add a scenario (see [Test harnesses](testing.md)).
|
||||
|
||||
@@ -0,0 +1,286 @@
|
||||
# Command schemas
|
||||
|
||||
Every command has an input schema and an output schema (JSON Schema 2020-12), generated by
|
||||
`schemars` from two Rust types:
|
||||
|
||||
- a **contract**: the parameters type, checked in front of the handler (see
|
||||
`crates/engine/src/schema/mod.rs`); with the session's check level at warn or strict, a
|
||||
misspelt key or a wrong-typed value is reported or refused instead of silently ignored;
|
||||
- a **reply type**: what a successful call returns.
|
||||
|
||||
`engine.describe` and the MCP tool `describe_command` serve both, so agents and scripts read a
|
||||
command's exact parameters instead of guessing from prose.
|
||||
|
||||
## When the check fails
|
||||
|
||||
The tests in `crates/engine/src/schema/coverage_tests.rs` say what to do. A command without
|
||||
schemas is only a warning for now (the test prints the list); it will become required once the
|
||||
PR backlog is down, so give new commands both schemas now. Set `SOLIDCRAFT_SCHEMA_RULE_STRICT=1`
|
||||
to make the missing-schema check fail today.
|
||||
|
||||
- **"`x.y` has no input or output schema"** (or "no input schema", "no output schema"): a new
|
||||
command. Give it what it lacks, as below. Don't add it to `schema/unconverted.txt`: that list
|
||||
holds only the commands that predate the rule.
|
||||
- **"`x.y` has its schemas now: remove its line from unconverted.txt"**: you converted an old
|
||||
command. Delete its line.
|
||||
- **"`x.y` is a planned! placeholder, which needs no line in unconverted.txt"**: delete its line.
|
||||
- **"unconverted.txt names commands that don't exist"**: a command was removed or renamed.
|
||||
Delete its line.
|
||||
- **"unconverted.txt must be sorted, each id once"**: keep the list in byte order
|
||||
(`LC_ALL=C sort`).
|
||||
- **"`x.y` ran with an unknown key in strict"** or **"`x.y` should refuse with …"**: it has a
|
||||
contract, but its `run` isn't `crate::checked!("x.y", <its handler>)` (step 5): the wrapper is
|
||||
missing or names another command's id.
|
||||
|
||||
## Writing a contract
|
||||
|
||||
### 1. List the keys the handler reads
|
||||
|
||||
The contract's keys are exactly the keys the handler reads, no more and no fewer. Read the
|
||||
handler and everything it passes `p` to:
|
||||
|
||||
- direct reads: `p.get("k")`, `params::expr(p, "k")`, `req_expr`, `str_`, `num`, `bool_`, `vec3`,
|
||||
`string_list` and the like;
|
||||
- `features::add_feature` (every feature command) reads `name`, `body_name`, `body_names` and
|
||||
`participants`: take them with [`FeatureCommon`](#shared-types). What `participants` changes
|
||||
depends on the feature, and for some it changes nothing in the result: check where the
|
||||
feature is evaluated (`crates/doc/src/eval.rs`). `participants` takes its description from
|
||||
`FeatureCommon`, shared by every feature, so say what it does for this one elsewhere: where it
|
||||
and another key override each other (a primitive's `targets` gives way to it; an extrude's
|
||||
`targets` overrides it), in that key's description; where it has no effect, in the summary.
|
||||
For the primitives (box, cylinder, sphere, torus) `add_feature` also reads `targets`; a new
|
||||
primitive must join the `primitive` match in `add_feature_in`, and its contract declares
|
||||
`targets`. The commands that combine bodies read `operation` (and usually `targets`) in their
|
||||
own handlers;
|
||||
- keys read around the command: `frames::to_local` reads `body` (and `face_body`) for commands
|
||||
that take picks;
|
||||
- `live`, for a command with `.live(…)` (today `sketch.drag` and `sketch.edit_text`): a call
|
||||
with `live: true` goes to the live runner, which `checked!` doesn't wrap, so live calls aren't
|
||||
checked yet. Leave those two on the list until the engine checks the live path too. A new
|
||||
command that needs a live runner still gets its contract (declare `live` as an optional
|
||||
`bool`); say in its pull request that its live calls aren't checked.
|
||||
|
||||
A key the handler reads only in some situations (thin-wall keys with `thin: true`) is still a
|
||||
field; its description says when it applies. Contracts don't check combinations of keys
|
||||
(`sketch` with `face`); the handler does.
|
||||
|
||||
### 2. Put it in the module for its command file
|
||||
|
||||
Contracts for `cmd/features.rs` live in `schema/features.rs`, for `cmd/inspect.rs` in
|
||||
`schema/inspect.rs`, and so on, each with its tests in a file next to it
|
||||
(`schema/features_tests.rs`). For a command file's first contract, create the module and add its
|
||||
`pub mod` line, sorted, under "The contracts, by command file" in `schema/mod.rs`. A new module
|
||||
starts like this (keep the imports it uses):
|
||||
|
||||
```rust
|
||||
//! Contracts and result types for the commands in `cmd/wedge.rs`.
|
||||
|
||||
use schemars::JsonSchema;
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use super::types::{FeatureCommon, LengthExpr, OperationParam, PointExpr3, StringList, feature_common, present};
|
||||
|
||||
// …the parameters and reply types…
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "wedge_tests.rs"]
|
||||
mod tests;
|
||||
```
|
||||
|
||||
### 3. Write the parameters type
|
||||
|
||||
```rust
|
||||
/// Add a wedge primitive.
|
||||
#[derive(Debug, Deserialize, JsonSchema)]
|
||||
#[schemars(deny_unknown_fields)]
|
||||
pub struct WedgeParams {
|
||||
/// Size along X.
|
||||
pub length: LengthExpr,
|
||||
/// Size along Y.
|
||||
pub width: LengthExpr,
|
||||
/// Height at the thick end, along Z.
|
||||
pub height: LengthExpr,
|
||||
/// The wedge's minimum corner. Default: the origin.
|
||||
#[serde(default, deserialize_with = "present", skip_serializing_if = "Option::is_none")]
|
||||
#[schemars(with = "PointExpr3")]
|
||||
pub corner: Option<PointExpr3>,
|
||||
/// How the wedge combines with existing bodies. Default: new.
|
||||
#[serde(default)]
|
||||
pub operation: Option<OperationParam>,
|
||||
/// The bodies a join, cut or intersect works on. Default: every body in the feature's
|
||||
/// component, not its sub-components (an intersect removes those the wedge doesn't meet).
|
||||
/// Ignored when `participants` lists bodies.
|
||||
#[serde(default)]
|
||||
pub targets: Option<StringList>,
|
||||
#[serde(flatten, deserialize_with = "feature_common")]
|
||||
pub common: FeatureCommon,
|
||||
}
|
||||
```
|
||||
|
||||
- **Descriptions are the documentation.** The struct's doc comment is the command's summary and
|
||||
each field's doc comment is its description in the schema, so put nothing else on those
|
||||
lines. Say the unit or form, the default (`Default: …`) and when the key is ignored. Write for
|
||||
someone who has only the schema. Check what each value does where the feature is evaluated
|
||||
(`crates/doc/src/eval.rs` and the kernel), not only how the handler reads it: a limit the
|
||||
evaluation enforces (a torus's `minor` must be less than its `major`) or a key it ignores
|
||||
belongs in the description.
|
||||
- **Required or optional** follows the handler: a key it can't do without is a plain field;
|
||||
any other is `#[serde(default)] Option<T>`.
|
||||
- **`null`:** the common readers (`str_`, `num`, `bool_`, `expr`, `string_list`) read `null` as
|
||||
absent, which a plain `#[serde(default)] Option<T>` matches. Readers that parse `p.get(k)`
|
||||
themselves (`features::feature_sketch`, `features::profiles`, the pick lists) refuse `null`;
|
||||
for those keys use `#[serde(default, deserialize_with = "present", skip_serializing_if =
|
||||
"Option::is_none")]` with `#[schemars(with = "T")]`. The key may then be absent but not
|
||||
`null`, the schema has no `null` branch, and `skip_serializing_if` stops schemars publishing
|
||||
`default: null`, which the schema itself would refuse.
|
||||
- **Use a shared type** for every value it fits (below) rather than `String` or `f64`: shared
|
||||
types accept exactly what the handlers' readers accept, give the handlers' messages, and
|
||||
publish the matching schema.
|
||||
|
||||
### 4. Write the reply type
|
||||
|
||||
A feature command whose handler returns `add_feature`'s reply unchanged returns
|
||||
`FeatureResult` (in `schema/features.rs`): use it, don't copy it. A command that returns
|
||||
anything else, including a feature command that reshapes the reply (`solid.thread` returns
|
||||
`{feature, name, thread}`), gets its own reply type:
|
||||
|
||||
```rust
|
||||
/// What `wedge.measure` returns.
|
||||
#[derive(Debug, Serialize, JsonSchema)]
|
||||
pub struct WedgeMeasureResult {
|
||||
/// The wedge's volume, in mm³.
|
||||
pub volume: f64,
|
||||
/// In the warn parameter-check mode: what the call's parameters got wrong.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub param_warnings: Vec<String>,
|
||||
}
|
||||
```
|
||||
|
||||
- Build the handler's reply from the type (`serde_json::to_value(reply)`), as `add_feature_in`
|
||||
builds `FeatureResult`, rather than a `json!` literal next to it. Then the schema can't drift
|
||||
from what the command returns.
|
||||
- Warn mode adds `param_warnings` to a successful reply that is a JSON object, so an object
|
||||
reply type keeps that field. A reply that isn't an object (a list) is returned unchanged and
|
||||
has no such field.
|
||||
|
||||
### 5. Register them
|
||||
|
||||
In the command file, next to its `COMMANDS`:
|
||||
|
||||
```rust
|
||||
use crate::schema::features::FeatureResult;
|
||||
use crate::schema::wedge::WedgeParams;
|
||||
use crate::schema::{TypedContract, output_schema_for};
|
||||
|
||||
const WEDGE_CONTRACT: TypedContract<WedgeParams> = TypedContract::new();
|
||||
|
||||
// in COMMANDS:
|
||||
CommandSpec::new("solid.wedge", "Wedge", crate::checked!("solid.wedge", prim_wedge))
|
||||
.at("SOLID", "CREATE")
|
||||
.icon("wedge")
|
||||
.contract(&WEDGE_CONTRACT)
|
||||
.output(output_schema_for::<FeatureResult>)
|
||||
.params("length, width, height: expr; corner?: [x,y,z] …"),
|
||||
```
|
||||
|
||||
`.contract` and `.output` publish the schemas, and `checked!` checks each call against the
|
||||
contract that its id publishes, in front of the handler, so what is checked is always what is
|
||||
published. The handler itself doesn't change: it still reads the raw parameters. Keep the
|
||||
`.params(…)` prose: the command list, the palette and `describe_command`'s `params` still show
|
||||
it, until it is generated from the schemas.
|
||||
|
||||
### 6. Test it
|
||||
|
||||
In the module's tests file:
|
||||
|
||||
- **`schemas_and_the_key_check_agree`:** add `assert_keys(input_schema_for::<WedgeParams>(),
|
||||
&[…])` with the keys, sorted, `FeatureCommon`'s included. The keys then change only on
|
||||
purpose.
|
||||
- **`strict_keeps_todays_errors`:** for calls the handler already refuses (a malformed value,
|
||||
an unknown spelling), add cases to `assert_strict_keeps_todays_errors`: strict must give the
|
||||
same message as the handler. Leave out the cases its documentation lists as different on
|
||||
purpose, such as a missing required key (strict says `` `length` is required ``, the handler
|
||||
something else).
|
||||
- **A reply test:** run the command and check the reply matches its output schema with
|
||||
`conforms` (`replies_match_the_output_schema` in `schema/features_tests.rs` shows how; its
|
||||
misspelt-key call checks that a reply carrying warnings still matches). Run every form the
|
||||
command takes (each alternative key, such as `corner` or `center`, each `operation`, one name
|
||||
and a list of names) in Warn, and check those replies have no `param_warnings`: Warn runs a
|
||||
call the contract refuses anyway, so a matching reply doesn't show the contract accepts the
|
||||
call. A reply that isn't an object never carries `param_warnings`; run those calls in Strict,
|
||||
where a refused call is an error.
|
||||
|
||||
`schema_defaults_are_valid` checks the `default` of each top-level parameter on its own; a
|
||||
nested type's defaults are yours to check.
|
||||
|
||||
### 7. Converting a command that already exists
|
||||
|
||||
Do steps 1 to 6, remove its line from `schema/unconverted.txt`, then check the contract against
|
||||
every caller the tests have: run the whole suite with the audit recorder and summarise it.
|
||||
|
||||
```text
|
||||
mkdir -p audit
|
||||
SOLIDCRAFT_PARAM_AUDIT=$PWD/audit cargo test --workspace --no-fail-fast --features solidcraft-engine/param-audit
|
||||
python3 scripts/param_audit.py audit
|
||||
```
|
||||
|
||||
The recorder doesn't create the directory, and it writes nothing if it can't, so check the
|
||||
summary's first line says at least one file. Each line under "SUCCEEDS TODAY, STRICT WOULD
|
||||
REFUSE" is a call that works today and that strict would refuse: a form the contract is
|
||||
missing (fix the contract) or a caller's mistake, such as a misspelt key (fix the caller). Put
|
||||
the summary in the pull request.
|
||||
|
||||
## Shared types
|
||||
|
||||
In `crates/engine/src/schema/types.rs`. Each accepts exactly what its reader accepts, most by
|
||||
calling that reader.
|
||||
|
||||
| Type | For | Matches |
|
||||
| --- | --- | --- |
|
||||
| `LengthExpr`, `AngleExpr` | a number or an expression (`"w / 2"`, `"30 deg"`) | `params::expr` |
|
||||
| `Point3`, `Point2` | `[x, y, z]` / `[x, y]` or `{x, y, z}` of numbers, in mm | `params::vec3`, `params::vec2` |
|
||||
| `PointExpr3` | a placement point whose coordinates may be expressions | `features::point_param` |
|
||||
| `IdOrName` | a feature by id or name (extrude's `sketch`) | `features::feature_sketch` |
|
||||
| `SketchRef` | a sketch by id or name, with `features::sketch_id`'s message | `features::sketch_id` |
|
||||
| `StringList` | one name or a list of names | `params::string_list` |
|
||||
| `OperationParam` | new, join, cut, intersect and their spellings | `Operation::parse` |
|
||||
| `FaceRef` | a face by a point on it or its name | `face::face_ref` |
|
||||
| `FaceRefs` | the `faces` list of shell, draft, offset face, split face and `face.delete` (not `solid.replace_face`, whose messages differ) | `features::face_points` |
|
||||
| `EdgeRef`, `EdgeRefs` | an edge by a point on it or `{body, index}`; fillet's and chamfer's `edges` | `features::edge_points` |
|
||||
| `ProfilesParam` | which sketch profiles | `features::profiles` |
|
||||
| `FeatureCommon` | `name`, `body_name`, `body_names`, `participants`, flattened | `features::add_feature` |
|
||||
|
||||
### When no shared type fits
|
||||
|
||||
Add one to `types.rs`, next to the reader it mirrors:
|
||||
|
||||
- Its `Deserialize` calls the **handler's own reader**, so "the contract accepts it" means "the
|
||||
handler accepts it". Don't re-implement the parsing. Many readers are private to their command
|
||||
file or need the design (`features::point_param` takes the `Session`). Split out the part that
|
||||
only looks at the JSON as a `pub(crate)` function, have the handler call it, and re-export it
|
||||
from `cmd/mod.rs`, as `face_ref_form` and `edge_ref_form` are; whether the value finds
|
||||
something in the design stays in the handler.
|
||||
- On failure it gives the **handler's message**. Wrap a message that is already a whole
|
||||
sentence, such as the handler's ``unknown operation `x` (…)``, in `whole(…)`, and the check
|
||||
passes it on as it is. Otherwise start it with "must": the check names the key, so "must be a
|
||||
point [x, y, z] of numbers" becomes ``"`corner` must be a point [x, y, z] of numbers"``. (Any
|
||||
other message comes out as ``"`corner`: …"``.)
|
||||
- Its `JsonSchema` describes the same values, with a description. Where JSON Schema can't say
|
||||
something the reader checks (ignoring case, a byte length), say it in the description.
|
||||
- Test both: values the reader accepts and refuses are accepted and refused by the type, and
|
||||
they match the schema (the `conforms` helper in `schema/tests.rs`; see
|
||||
`face_and_edge_refs_match_the_pick_readers`).
|
||||
- A handler that reads a key more leniently than any type can (a wrong-typed value read as if
|
||||
the key were absent) gets the stricter type: that difference is what warn and strict report.
|
||||
|
||||
## Checklist
|
||||
|
||||
1. The contract's keys are exactly the handler's reads, including `add_feature`'s, the frame
|
||||
mapping's and `live`.
|
||||
2. The struct and every field have descriptions, with defaults and when a key is ignored.
|
||||
3. Shared types wherever they fit; a new type calls the handler's reader and gives its message.
|
||||
4. `FeatureResult` for a command that returns `add_feature`'s reply unchanged, else a reply type
|
||||
the handler builds its reply from.
|
||||
5. `checked!`, `.contract` and `.output` on the `CommandSpec`, and the `.params` prose kept.
|
||||
6. `assert_keys`, error-message cases, a reply test.
|
||||
7. For an existing command: its `unconverted.txt` line removed and the audit summary in the PR.
|
||||
+3
-3
@@ -65,8 +65,8 @@ temporary file by the app and read back by the server, so both must run on the s
|
||||
|
||||
### Parameter checking
|
||||
|
||||
Commands with a typed parameter contract (`solid.box`, `solid.extrude` and `engine.describe`
|
||||
so far; `describe_command` shows a command's schemas) can be checked as they are called.
|
||||
Commands with a typed parameter contract (every new command, and more of the older ones as they
|
||||
are converted; `describe_command` shows a command's schemas) can be checked as they are called.
|
||||
`--param-check off|warn|strict` sets the level for a headless session, including a script or
|
||||
recipe given with `--in`; the default is `off`, which changes nothing. `warn` runs the call and
|
||||
adds `param_warnings` to its result, such as ``unknown key `operaton`; did you mean
|
||||
@@ -254,7 +254,7 @@ The `skill_reference` tool returns these documents from the copies built into So
|
||||
| Tool | Arguments | What it does |
|
||||
|---|---|---|
|
||||
| `list_commands` | `filter?` | The command catalog: id, label, tab/panel, shortcut, parameter docs, enabled now. `filter` matches id, label, tab and panel. |
|
||||
| `describe_command` | `id` | One command's parameter docs (`params`) and, for commands with a typed contract (`solid.box`, `solid.extrude` and `engine.describe` so far), the JSON Schemas (2020-12) of its parameters (`input`) and of a successful result (`output`); `null` for the others. The same as `execute` `engine.describe {command}`. |
|
||||
| `describe_command` | `id` | One command's parameter docs (`params`) and, for commands with a typed contract (every new command, and more of the older ones as they are converted), the JSON Schemas (2020-12) of its parameters (`input`) and of a successful result (`output`); `null` for the others. The same as `execute` `engine.describe {command}`. |
|
||||
| `execute` | `command`, `params?`, `verbose?` (default true) | Run any command with JSON parameters (`list_commands` shows them). Failures leave the design unchanged. A feature command replies with the feature (`feature`, `name`), every body (`bodies`) and the bodies it created, changed and removed (`created_bodies`, `changed_bodies`, `removed_bodies`). `verbose: false` uses the same compact reply as `batch`. |
|
||||
| `batch` | `commands: [{command, params}]`, `rollback?` (default true), `verbose?` (default true), `timings?`, `profile?`, `coalesce_parameters?` (default false) | Run steps in order, stop at the first error and undo the completed steps. The reply names the failing step (`failed.index`, `failed.command`, `failed.error`), every result before it, `rolled_back` (true only when the design really went back to where the batch started), `undone`, `applied` (the indices of the steps whose changes are in the design) and `not_run` (the steps never sent). A step may name a tool instead of a command, with that tool's arguments as `params`: `set_parameter` (runs `parameters.change`; `value` or `expression`), `list_commands` (`engine.commands`, filtered by `filter` as the tool filters it), `describe_command` (`engine.describe`), `execute` (`{command, params}`), `measure`, `inspect_design` (`document.inspect`: each sketch as its counts, not its full contents, and no joints), `undo`, `redo`, `export`, `save`, `open`, `new_design` and `crash_reports`. `screenshot`, `body_topology` (use `model.edges` and `model.faces`), `batch`, `pending`, `running` and `skill_reference` can't be steps: such a batch fails before anything is sent. `verbose: false` keeps the reply small in large designs: feature steps leave out the full body list (`created_bodies`, `changed_bodies`, `removed_bodies` stay) and sketch steps the ids they repeat outside `created`. Bridged to an app that has `engine.batch`, the whole batch is one control request (one frame of the app). Older apps, and apps showing the agent cursor, get the steps one by one. `timings: true` adds `timings: [{index, command, ms}]` for every step run, the failing one included, and `total_ms`. `profile: true` also adds each step's `profile: {boolean_ms, tessellate_ms, other_ms}`, its time in kernel booleans, in tessellation and the rest. Steps are timed where they run: in the app when bridged. An app without `engine.batch` is timed step by step, without `profile`. `coalesce_parameters: true` sets each run of consecutive plain parameter edits (`parameters.change` or `set_parameter` with a name and a value, or `changes`) together, so the design rebuilds once per run instead of once per edit. Steps between runs see the design up to date. Each step keeps its own result (`coalesced`: the run's length), and the run is one undo step. The run skips the states between its edits, so an edit refused on its own can go through with the run's later ones. A refused run goes step by step, naming the refused step. |
|
||||
| `inspect_design` | `measure?` (default true), `sketches?` (default true), `joints?` (default true) | Design properties (`metadata`: project, author… and other tools' keys; set them with `execute` `document.metadata.set`), parameters, timeline (type, suppressed, rolled back, error, warning, timing), bodies (volume, area, centre of mass, bbox, face/edge/vertex counts, visible; `removed_later: true` while a sketch is being edited on a rolled-back timeline and a later feature deletes the body, which is then not shown), sketches (curves, points, constraints, profiles, DOF, solve status, visible), joints when the design has any (as `joint.inspect` gives them: each origin's occurrence, component and path, the snap as saved, the feature, face, edge or sketch curve (`sketch: {id, name, curve}`) its persistent names refer to, `resolves` and the `error` when it is lost; kind, offset, angle, flip, values, limits, expressions, motion links; `execute` `joint.inspect {joint}` gives one joint), selection, undo depth. Hide helper sketches, bodies or construction planes with `execute` `browser.visibility` (`{items, visible}` or `{folder, visible}`), and joint glyphs with `{joints: [id or name], visible}` or `{folder: "joints", visible}` (a hidden joint still constrains and drives; `joint.list` reports each joint's `visible`). |
|
||||
|
||||
Reference in New Issue
Block a user