From d79ec9502881ffa4247a8dfa716d77b6057c361c Mon Sep 17 00:00:00 2001 From: github-ops Date: Sat, 20 Jun 2026 19:59:55 +0000 Subject: [PATCH] Add schema quality checks --- apps/admin-api/src/service.rs | 51 +++++++- crates/crank-core/src/lib.rs | 3 +- crates/crank-core/src/tool_quality.rs | 127 +++++++++++++++++++ crates/crank-core/tests/unit/tool_quality.rs | 71 ++++++++++- 4 files changed, 242 insertions(+), 10 deletions(-) diff --git a/apps/admin-api/src/service.rs b/apps/admin-api/src/service.rs index 8774f82..daa8fc9 100644 --- a/apps/admin-api/src/service.rs +++ b/apps/admin-api/src/service.rs @@ -12,8 +12,8 @@ use crank_core::{ NoopAuditSink, OperationId, OperationSecurityLevel, OperationStatus, OwnerOnlyPolicyEngine, PlatformApiKey, PlatformApiKeyId, PlatformApiKeyScope, PlatformApiKeyStatus, PolicyEngine, ProductEdition, Protocol, ResponseCachePolicy, SampleId, Samples, Secret, SecretId, SecretKind, - SecretStatus, Target, UsagePeriod, UserId, UserSessionId, WizardState, Workspace, WorkspaceId, - WorkspaceStatus, + SecretStatus, Target, ToolQualitySchemaKind, ToolQualitySchemaNode, UsagePeriod, UserId, + UserSessionId, WizardState, Workspace, WorkspaceId, WorkspaceStatus, }; use crank_mapping::{JsonPathRoot, MappingSet, infer_mapping_from_samples}; use crank_registry::{ @@ -32,7 +32,7 @@ use crank_runtime::{ PreparedRequest, ResolvedAuth, RuntimeError, RuntimeExecutor, RuntimeOperation, RuntimeRequestContext, SecretCrypto, }; -use crank_schema::Schema; +use crank_schema::{Schema, SchemaKind}; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; use sha2::{Digest, Sha256}; @@ -1452,10 +1452,15 @@ impl AdminService { self.validate_operation_payload(&payload)?; self.ensure_workspace_exists(workspace_id).await?; - Ok(crank_core::analyze_tool_identity_quality( - &payload.name, - &payload.tool_description, - )) + let mut findings = + crank_core::analyze_tool_identity_quality(&payload.name, &payload.tool_description) + .findings; + let input_schema = tool_quality_schema_node(&payload.input_schema); + findings.extend( + crank_core::analyze_tool_schema_quality("input_schema", &input_schema).findings, + ); + + Ok(crank_core::ToolQualityReport::new(findings)) } #[instrument(skip(self, payload), fields(operation_id = %operation_id.as_str(), protocol = ?payload.operation.protocol))] @@ -3795,6 +3800,38 @@ fn validate_response_cache_policy( Ok(()) } +fn tool_quality_schema_node(schema: &Schema) -> ToolQualitySchemaNode { + ToolQualitySchemaNode { + kind: tool_quality_schema_kind(&schema.kind), + description: schema.description.clone(), + fields: schema + .fields + .iter() + .map(|(name, field)| (name.clone(), tool_quality_schema_node(field))) + .collect(), + items: schema + .items + .as_deref() + .map(tool_quality_schema_node) + .map(Box::new), + enum_values: schema.enum_values.clone(), + } +} + +fn tool_quality_schema_kind(kind: &SchemaKind) -> ToolQualitySchemaKind { + match kind { + SchemaKind::Object => ToolQualitySchemaKind::Object, + SchemaKind::Array => ToolQualitySchemaKind::Array, + SchemaKind::String => ToolQualitySchemaKind::String, + SchemaKind::Integer => ToolQualitySchemaKind::Integer, + SchemaKind::Number => ToolQualitySchemaKind::Number, + SchemaKind::Boolean => ToolQualitySchemaKind::Boolean, + SchemaKind::Enum => ToolQualitySchemaKind::Enum, + SchemaKind::Null => ToolQualitySchemaKind::Null, + SchemaKind::Oneof => ToolQualitySchemaKind::Oneof, + } +} + #[cfg(test)] #[allow(clippy::items_after_test_module)] mod tests { diff --git a/crates/crank-core/src/lib.rs b/crates/crank-core/src/lib.rs index 777cde0..f0652ee 100644 --- a/crates/crank-core/src/lib.rs +++ b/crates/crank-core/src/lib.rs @@ -63,6 +63,7 @@ pub use operation::{ pub use protocol::{AuthKind, ExportMode, HttpMethod, Protocol}; pub use secret::{Secret, SecretKind, SecretStatus, SecretVersion}; pub use tool_quality::{ - ToolQualityFinding, ToolQualityReport, ToolQualitySeverity, analyze_tool_identity_quality, + ToolQualityFinding, ToolQualityReport, ToolQualitySchemaKind, ToolQualitySchemaNode, + ToolQualitySeverity, analyze_tool_identity_quality, analyze_tool_schema_quality, }; pub use workspace::{Workspace, WorkspaceStatus}; diff --git a/crates/crank-core/src/tool_quality.rs b/crates/crank-core/src/tool_quality.rs index cd739ec..c05d3a4 100644 --- a/crates/crank-core/src/tool_quality.rs +++ b/crates/crank-core/src/tool_quality.rs @@ -5,6 +5,8 @@ use crate::ToolDescription; const MAX_TOOL_NAME_LEN: usize = 64; const MIN_DESCRIPTION_CHARS: usize = 40; const GENERIC_TOOL_NAMES: &[&str] = &["call_api", "manage", "execute", "request"]; +const MULTI_ACTION_FIELD_NAMES: &[&str] = &["action", "mode", "operation", "type"]; +const ENUM_LIKE_FIELD_NAMES: &[&str] = &["mode", "type", "status", "category"]; #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -31,6 +33,31 @@ pub struct ToolQualityReport { pub findings: Vec, } +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ToolQualitySchemaKind { + Object, + Array, + String, + Integer, + Number, + Boolean, + Enum, + Null, + Oneof, +} + +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct ToolQualitySchemaNode { + pub kind: ToolQualitySchemaKind, + #[serde(skip_serializing_if = "Option::is_none")] + pub description: Option, + pub fields: Vec<(String, ToolQualitySchemaNode)>, + #[serde(skip_serializing_if = "Option::is_none")] + pub items: Option>, + pub enum_values: Vec, +} + impl ToolQualityReport { pub fn new(findings: Vec) -> Self { let blocking = findings @@ -45,6 +72,31 @@ impl ToolQualityReport { } } +impl ToolQualitySchemaNode { + pub fn object(description: Option<&str>, fields: Vec<(&str, ToolQualitySchemaNode)>) -> Self { + Self { + kind: ToolQualitySchemaKind::Object, + description: description.map(ToOwned::to_owned), + fields: fields + .into_iter() + .map(|(name, schema)| (name.to_owned(), schema)) + .collect(), + items: None, + enum_values: Vec::new(), + } + } + + pub fn string(description: Option<&str>) -> Self { + Self { + kind: ToolQualitySchemaKind::String, + description: description.map(ToOwned::to_owned), + fields: Vec::new(), + items: None, + enum_values: Vec::new(), + } + } +} + pub fn analyze_tool_identity_quality( tool_name: &str, tool_description: &ToolDescription, @@ -114,6 +166,15 @@ pub fn analyze_tool_identity_quality( ToolQualityReport::new(findings) } +pub fn analyze_tool_schema_quality( + root_path: &str, + schema: &ToolQualitySchemaNode, +) -> ToolQualityReport { + let mut findings = Vec::new(); + analyze_schema_node(root_path, schema, &mut findings); + ToolQualityReport::new(findings) +} + fn is_safe_tool_name(value: &str) -> bool { let mut chars = value.chars(); let Some(first) = chars.next() else { @@ -126,6 +187,72 @@ fn is_safe_tool_name(value: &str) -> bool { chars.all(|ch| ch.is_ascii_lowercase() || ch.is_ascii_digit() || ch == '_') } +fn analyze_schema_node( + path: &str, + schema: &ToolQualitySchemaNode, + findings: &mut Vec, +) { + if schema.kind == ToolQualitySchemaKind::Object && schema.fields.is_empty() { + findings.push(finding( + ToolQualitySeverity::Warning, + "schema_object_without_fields", + "Схема объекта не содержит параметров.", + Some("Добавьте только те параметры, которые действительно нужны агенту для вызова инструмента."), + Some(path), + )); + } + + for (field_name, field_schema) in &schema.fields { + let field_path = format!("{path}.fields.{field_name}"); + + if is_missing_description(field_schema) { + findings.push(finding( + ToolQualitySeverity::Warning, + "schema_field_missing_description", + "У параметра нет описания.", + Some("Опишите назначение параметра понятным языком, чтобы модель не угадывала его смысл."), + Some(&format!("{field_path}.description")), + )); + } + + if MULTI_ACTION_FIELD_NAMES.contains(&field_name.as_str()) { + findings.push(finding( + ToolQualitySeverity::Warning, + "schema_multi_action_parameter", + "Параметр выглядит как переключатель нескольких действий.", + Some("Лучше разделить разные действия на отдельные MCP инструменты с узкими именами и схемами."), + Some(&field_path), + )); + } + + if ENUM_LIKE_FIELD_NAMES.contains(&field_name.as_str()) + && field_schema.kind == ToolQualitySchemaKind::String + && field_schema.enum_values.is_empty() + { + findings.push(finding( + ToolQualitySeverity::Info, + "schema_enum_recommended", + "Для параметра с ограниченным набором значений лучше указать enum.", + Some("Задайте допустимые значения явно, чтобы агент не придумывал варианты."), + Some(&field_path), + )); + } + + analyze_schema_node(&field_path, field_schema, findings); + } + + if let Some(items) = &schema.items { + analyze_schema_node(&format!("{path}.items"), items, findings); + } +} + +fn is_missing_description(schema: &ToolQualitySchemaNode) -> bool { + schema + .description + .as_deref() + .is_none_or(|description| description.trim().is_empty()) +} + fn finding( severity: ToolQualitySeverity, code: &str, diff --git a/crates/crank-core/tests/unit/tool_quality.rs b/crates/crank-core/tests/unit/tool_quality.rs index 4252cdf..6ea01dc 100644 --- a/crates/crank-core/tests/unit/tool_quality.rs +++ b/crates/crank-core/tests/unit/tool_quality.rs @@ -1,6 +1,6 @@ use crank_core::{ - ToolDescription, ToolQualityFinding, ToolQualityReport, ToolQualitySeverity, - analyze_tool_identity_quality, + ToolDescription, ToolQualityFinding, ToolQualityReport, ToolQualitySchemaNode, + ToolQualitySeverity, analyze_tool_identity_quality, analyze_tool_schema_quality, }; #[test] @@ -114,6 +114,73 @@ fn warns_about_short_description() { )); } +#[test] +fn warns_about_object_schema_without_fields() { + let schema = ToolQualitySchemaNode::object(None, vec![]); + + let report = analyze_tool_schema_quality("input_schema", &schema); + + assert!(has_finding( + &report, + "schema_object_without_fields", + ToolQualitySeverity::Warning + )); +} + +#[test] +fn warns_about_parameter_without_description() { + let schema = ToolQualitySchemaNode::object( + Some("Входные параметры"), + vec![("customer_id", ToolQualitySchemaNode::string(None))], + ); + + let report = analyze_tool_schema_quality("input_schema", &schema); + + assert!(has_finding( + &report, + "schema_field_missing_description", + ToolQualitySeverity::Warning + )); +} + +#[test] +fn warns_about_multi_action_parameter_names() { + let schema = ToolQualitySchemaNode::object( + Some("Входные параметры"), + vec![( + "action", + ToolQualitySchemaNode::string(Some("Что нужно сделать")), + )], + ); + + let report = analyze_tool_schema_quality("input_schema", &schema); + + assert!(has_finding( + &report, + "schema_multi_action_parameter", + ToolQualitySeverity::Warning + )); +} + +#[test] +fn recommends_enum_for_mode_like_string_parameter() { + let schema = ToolQualitySchemaNode::object( + Some("Входные параметры"), + vec![( + "mode", + ToolQualitySchemaNode::string(Some("Режим поиска клиента")), + )], + ); + + let report = analyze_tool_schema_quality("input_schema", &schema); + + assert!(has_finding( + &report, + "schema_enum_recommended", + ToolQualitySeverity::Info + )); +} + fn has_finding(report: &ToolQualityReport, code: &str, severity: ToolQualitySeverity) -> bool { report .findings