Add schema quality checks

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 {

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};

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<ToolQualityFinding>,
 }
 
+#[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<String>,
+    pub fields: Vec<(String, ToolQualitySchemaNode)>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub items: Option<Box<ToolQualitySchemaNode>>,
+    pub enum_values: Vec<String>,
+}
+
 impl ToolQualityReport {
     pub fn new(findings: Vec<ToolQualityFinding>) -> 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<ToolQualityFinding>,
+) {
+    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,

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