//! Tool description quality lint rules.
//!
//! Each rule inspects one tool object from a `tools/list` response and
//! emits a [`Finding`] when it triggers. Rules are stable, individually
//! addressable by ID, and toggleable, so CI can pin a subset.

use std::io::Write;

use serde::{Deserialize, Serialize};
use serde_json::Value;

/// Quality verdict for a single rule against a single tool.
///
/// Tools that satisfy every rule produce a synthetic `Pass` finding so
/// the report always represents every checked tool.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum Severity {
    /// Tool meets the quality bar for this rule.
    Pass,
    /// Imperfect but workable. The agent will probably still pick this
    /// tool correctly; LLM-side accuracy may suffer.
    Warning,
    /// Violates a research-backed quality rule. The agent will likely
    /// struggle to use this tool reliably.
    Critical,
}

/// One quality finding from one rule against one tool.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct Finding {
    /// Tool name from the catalog.
    pub tool_name: String,
    /// Stable rule ID (`DESC-001` through `DESC-013`).
    pub rule_id: &'static str,
    /// Quality verdict.
    pub severity: Severity,
    /// Human-readable explanation of what this rule found.
    pub message: String,
    /// Optional suggested fix.
    pub suggestion: Option<String>,
}

/// Aggregate report from running every rule against every tool in a
/// catalog.
#[derive(Debug, Clone)]
pub struct LintReport {
    /// Findings in tool-then-rule order.
    pub findings: Vec<Finding>,
    /// Number of tools examined.
    pub tools_checked: usize,
}

impl LintReport {
    /// Count of findings at the [`Severity::Critical`] level.
    pub fn critical_count(&self) -> usize {
        self.findings
            .iter()
            .filter(|f| f.severity == Severity::Critical)
            .count()
    }

    /// Count of findings at the [`Severity::Warning`] level.
    pub fn warning_count(&self) -> usize {
        self.findings
            .iter()
            .filter(|f| f.severity == Severity::Warning)
            .count()
    }

    /// Count of [`Severity::Pass`] entries (one per clean tool).
    pub fn pass_count(&self) -> usize {
        self.findings
            .iter()
            .filter(|f| f.severity == Severity::Pass)
            .count()
    }

    /// True when any tool triggered at least one [`Severity::Critical`].
    pub fn has_critical(&self) -> bool {
        self.critical_count() > 0
    }
}

/// One quality rule. Implementations should be cheap to clone and
/// stateless across runs.
pub trait Rule: Send + Sync {
    /// Stable rule ID, e.g. `"DESC-001"`.
    fn id(&self) -> &'static str;
    /// Return a finding when the tool violates this rule. Return `None`
    /// when the rule is satisfied.
    fn check(&self, tool: &Value) -> Option<Finding>;
}

/// Construct the canonical set of all thirteen DESC-* rules.
pub fn default_rules() -> Vec<Box<dyn Rule>> {
    vec![
        Box::new(DescriptionTooShort),
        Box::new(DescriptionTooLong),
        Box::new(DescriptionDuplicatesName),
        Box::new(DescriptionMissingVerb),
        Box::new(DescriptionReferencesPosition),
        Box::new(ArgumentMissingDescription),
        Box::new(EnumArgWithoutValueDescriptions),
        Box::new(ArgDescriptionLongerThanTool),
        Box::new(NonTrivialSchemaWithoutExamples),
        Box::new(DescriptionMissingReturnFormat),
        Box::new(AnnotationHintNotBoolean),
        Box::new(ToolWithoutAnnotations),
        Box::new(StringArgEnumerableInDescription),
    ]
}

/// Run every rule against every tool in a `tools/list` catalog.
///
/// `catalog` is the parsed JSON-RPC response, expected to contain a
/// `tools` array. Tools without findings emit a [`Severity::Pass`] entry
/// so reporters can show "5 passed, 2 warnings, 1 critical" totals.
pub fn lint_catalog(catalog: &Value, rules: &[&dyn Rule]) -> LintReport {
    let tools = catalog
        .get("tools")
        .and_then(|v| v.as_array())
        .cloned()
        .unwrap_or_default();

    let mut findings = Vec::new();
    for tool in &tools {
        let name = tool_name(tool);
        let mut had_any = false;
        for rule in rules {
            if let Some(f) = rule.check(tool) {
                findings.push(f);
                had_any = true;
            }
        }
        if !had_any {
            findings.push(Finding {
                tool_name: name,
                rule_id: "PASS",
                severity: Severity::Pass,
                message: "tool description meets all quality rules".into(),
                suggestion: None,
            });
        }
    }
    LintReport {
        findings,
        tools_checked: tools.len(),
    }
}

/// Pretty-render a lint report. ASCII glyphs only so the output is safe
/// on every terminal AGENTS.md forbids em-dashes anywhere, including in
/// reporter output.
pub fn render_pretty(report: &LintReport, out: &mut dyn Write) -> std::io::Result<()> {
    writeln!(out, "Tool catalog quality check:")?;
    writeln!(out)?;
    let mut current_tool = "".to_string();
    for f in &report.findings {
        if f.tool_name != current_tool {
            current_tool = f.tool_name.clone();
            writeln!(out, "  Tool: {current_tool}")?;
        }
        let glyph = match f.severity {
            Severity::Pass => "[OK]",
            Severity::Warning => "[!!]",
            Severity::Critical => "[XX]",
        };
        writeln!(out, "    {glyph} {} ({})", f.message, f.rule_id)?;
        if let Some(s) = &f.suggestion {
            writeln!(out, "         Suggest: {s}")?;
        }
    }
    writeln!(out)?;
    writeln!(
        out,
        "Quality summary: {} critical, {} warnings, {} pass ({} tools checked)",
        report.critical_count(),
        report.warning_count(),
        report.pass_count(),
        report.tools_checked
    )?;
    Ok(())
}

fn tool_name(tool: &Value) -> String {
    tool.get("name")
        .and_then(|v| v.as_str())
        .unwrap_or("<unknown>")
        .to_string()
}

fn description(tool: &Value) -> &str {
    tool.get("description")
        .and_then(|v| v.as_str())
        .unwrap_or("")
}

fn finding(
    tool: &Value,
    rule_id: &'static str,
    severity: Severity,
    message: impl Into<String>,
    suggestion: Option<&str>,
) -> Finding {
    Finding {
        tool_name: tool_name(tool),
        rule_id,
        severity,
        message: message.into(),
        suggestion: suggestion.map(|s| s.to_string()),
    }
}

const COMMON_VERBS: &[&str] = &[
    "create",
    "get",
    "list",
    "delete",
    "fetch",
    "summarize",
    "search",
    "update",
    "find",
    "open",
    "close",
    "send",
    "receive",
    "load",
    "save",
    "parse",
    "format",
    "convert",
    "filter",
    "sort",
    "match",
    "validate",
    "compute",
    "calculate",
    "render",
    "generate",
    "import",
    "export",
    "publish",
    "subscribe",
    "watch",
    "read",
    "write",
    "edit",
    "remove",
    "add",
    "build",
    "run",
    "return",
    "returns",
    "respond",
    "responds",
    "describe",
    "show",
    "shows",
    "lookup",
];

const POSITIONAL_PHRASES: &[&str] = &[
    "above",
    "below",
    "previous tool",
    "this section",
    "see also",
    "next paragraph",
    "as shown above",
];

/// DESC-001: description shorter than 20 chars.
pub struct DescriptionTooShort;
impl Rule for DescriptionTooShort {
    fn id(&self) -> &'static str {
        "DESC-001"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let desc = description(tool);
        if desc.chars().count() < 20 && !desc.is_empty() {
            return Some(finding(
                tool,
                self.id(),
                Severity::Critical,
                format!("description is only {} chars, too terse to disambiguate", desc.chars().count()),
                Some("expand to at least 20 chars including the main verb and one disambiguating noun"),
            ));
        }
        if desc.is_empty() {
            return Some(finding(
                tool,
                self.id(),
                Severity::Critical,
                "description is empty",
                Some("add a 1-line description starting with the main verb"),
            ));
        }
        None
    }
}

/// DESC-002: description longer than 500 chars.
pub struct DescriptionTooLong;
impl Rule for DescriptionTooLong {
    fn id(&self) -> &'static str {
        "DESC-002"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let desc = description(tool);
        let n = desc.chars().count();
        if n > 500 {
            return Some(finding(
                tool,
                self.id(),
                Severity::Warning,
                format!("description is {n} chars, likely missing arg constraints in inputSchema"),
                Some(
                    "move parameter constraints into the inputSchema and keep description focused",
                ),
            ));
        }
        None
    }
}

/// DESC-003: description equals the tool name.
pub struct DescriptionDuplicatesName;
impl Rule for DescriptionDuplicatesName {
    fn id(&self) -> &'static str {
        "DESC-003"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let name = tool_name(tool);
        let desc = description(tool);
        if !desc.is_empty() && desc.trim().eq_ignore_ascii_case(name.trim()) {
            return Some(finding(
                tool,
                self.id(),
                Severity::Critical,
                "description duplicates the tool name and adds no signal",
                Some("describe the verb and intent, not just the name"),
            ));
        }
        None
    }
}

/// DESC-004: description does not contain any common verb.
pub struct DescriptionMissingVerb;
impl Rule for DescriptionMissingVerb {
    fn id(&self) -> &'static str {
        "DESC-004"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let desc = description(tool).to_lowercase();
        if desc.is_empty() {
            return None;
        }
        let has_verb = COMMON_VERBS.iter().any(|v| {
            desc.split_whitespace()
                .any(|w| w.trim_matches(|c: char| !c.is_alphanumeric()) == *v)
        });
        if !has_verb {
            return Some(finding(
                tool,
                self.id(),
                Severity::Warning,
                "description does not contain a common verb; agent cannot categorize read/write/delete",
                Some("rewrite to start with a clear verb like 'create', 'list', or 'summarize'"),
            ));
        }
        None
    }
}

/// DESC-005: description uses positional phrases that have no meaning in
/// a flat tool catalog (e.g. "see above").
pub struct DescriptionReferencesPosition;
impl Rule for DescriptionReferencesPosition {
    fn id(&self) -> &'static str {
        "DESC-005"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let desc_lower = description(tool).to_lowercase();
        for phrase in POSITIONAL_PHRASES {
            if desc_lower.contains(phrase) {
                return Some(finding(
                    tool,
                    self.id(),
                    Severity::Warning,
                    format!("description references '{phrase}' but agents have no spatial context"),
                    Some(
                        "rewrite without positional references; describe this tool self-contained",
                    ),
                ));
            }
        }
        None
    }
}

/// DESC-006: a required argument has no `description` field.
pub struct ArgumentMissingDescription;
impl Rule for ArgumentMissingDescription {
    fn id(&self) -> &'static str {
        "DESC-006"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let schema = tool.get("inputSchema")?;
        let properties = schema.get("properties")?.as_object()?;
        let empty: Vec<Value> = Vec::new();
        let required: Vec<&str> = schema
            .get("required")
            .and_then(|r| r.as_array())
            .unwrap_or(&empty)
            .iter()
            .filter_map(|v| v.as_str())
            .collect();
        for arg in required {
            if let Some(prop) = properties.get(arg) {
                let has_desc = prop
                    .get("description")
                    .and_then(|d| d.as_str())
                    .map(|s| !s.is_empty())
                    .unwrap_or(false);
                if !has_desc {
                    return Some(finding(
                        tool,
                        self.id(),
                        Severity::Warning,
                        format!("required argument '{arg}' has no description"),
                        Some(&format!(
                            "add a `description` to inputSchema.properties.{arg}"
                        )),
                    ));
                }
            }
        }
        None
    }
}

/// DESC-007: enum-typed argument lacks a description that mentions the
/// enum values.
pub struct EnumArgWithoutValueDescriptions;
impl Rule for EnumArgWithoutValueDescriptions {
    fn id(&self) -> &'static str {
        "DESC-007"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let properties = tool
            .get("inputSchema")
            .and_then(|s| s.get("properties"))
            .and_then(|p| p.as_object())?;
        for (arg, prop) in properties {
            let enum_values = prop.get("enum").and_then(|e| e.as_array());
            if let Some(values) = enum_values {
                let desc = prop
                    .get("description")
                    .and_then(|d| d.as_str())
                    .unwrap_or("");
                let value_strs: Vec<String> = values
                    .iter()
                    .filter_map(|v| v.as_str().map(|s| s.to_string()))
                    .collect();
                let mentions_any = value_strs
                    .iter()
                    .any(|v| desc.to_lowercase().contains(&v.to_lowercase()));
                if !mentions_any && !value_strs.is_empty() {
                    return Some(finding(
                        tool,
                        self.id(),
                        Severity::Warning,
                        format!("enum argument '{arg}' has values {value_strs:?} but description does not document them"),
                        Some(&format!("expand description to explain when to pick each enum value for '{arg}'")),
                    ));
                }
            }
        }
        None
    }
}

/// DESC-008: an argument description is longer than the tool description.
/// Inverted information density.
pub struct ArgDescriptionLongerThanTool;
impl Rule for ArgDescriptionLongerThanTool {
    fn id(&self) -> &'static str {
        "DESC-008"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let tool_desc_len = description(tool).chars().count();
        if tool_desc_len == 0 {
            return None;
        }
        let properties = tool
            .get("inputSchema")
            .and_then(|s| s.get("properties"))
            .and_then(|p| p.as_object())?;
        for (arg, prop) in properties {
            let arg_desc = prop
                .get("description")
                .and_then(|d| d.as_str())
                .unwrap_or("");
            if arg_desc.chars().count() > tool_desc_len {
                return Some(finding(
                    tool,
                    self.id(),
                    Severity::Warning,
                    format!("argument '{arg}' description ({} chars) is longer than the tool description ({tool_desc_len} chars)", arg_desc.chars().count()),
                    Some("expand the tool description so it carries the high-level intent"),
                ));
            }
        }
        None
    }
}

/// Keywords that signal a description documents what the tool returns.
/// Used by DESC-010 as a coarse, deterministic heuristic.
const RETURN_KEYWORDS: &[&str] = &[
    "return",
    "returns",
    "returning",
    "output",
    "outputs",
    "result",
    "results",
    "response",
    "responds",
    "yields",
    "produces",
    "emits",
    "gives back",
];

/// True when a property declares an inline example, either via the JSON
/// Schema `examples` array, a singular `example`, or a `default` value an
/// agent can copy. Keeping the set explicit avoids false negatives from
/// vendor-specific keys we do not recognize.
fn property_has_example(prop: &Value) -> bool {
    let has_examples_array = prop
        .get("examples")
        .and_then(|e| e.as_array())
        .map(|a| !a.is_empty())
        .unwrap_or(false);
    let has_singular_example = prop.get("example").is_some();
    let has_default = prop.get("default").is_some();
    has_examples_array || has_singular_example || has_default
}

/// True when the input schema is non-trivial enough that worked examples
/// meaningfully help an agent: more than one property, or any single
/// property that is required or not a plain string.
fn schema_is_non_trivial(schema: &Value) -> bool {
    let Some(properties) = schema.get("properties").and_then(|p| p.as_object()) else {
        return false;
    };
    if properties.len() > 1 {
        return true;
    }
    let empty: Vec<Value> = Vec::new();
    let required: Vec<&str> = schema
        .get("required")
        .and_then(|r| r.as_array())
        .unwrap_or(&empty)
        .iter()
        .filter_map(|v| v.as_str())
        .collect();
    for (name, prop) in properties {
        if required.contains(&name.as_str()) {
            return true;
        }
        let is_plain_string = prop
            .get("type")
            .and_then(|t| t.as_str())
            .map(|t| t == "string")
            .unwrap_or(false)
            && prop.get("enum").is_none();
        if !is_plain_string {
            return true;
        }
    }
    false
}

/// DESC-009: a tool with a non-trivial input schema provides no usage
/// examples. Anthropic's advanced-tool-use guidance reports that adding
/// 1-5 concise examples lifted accuracy from 72% to 90% on complex
/// parameter handling. Examples may live in an `examples` field on the
/// tool object or as `examples`/`example`/`default` values inside the
/// input schema; we accept any of these so well-documented tools pass.
pub struct NonTrivialSchemaWithoutExamples;
impl Rule for NonTrivialSchemaWithoutExamples {
    fn id(&self) -> &'static str {
        "DESC-009"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let schema = tool.get("inputSchema")?;
        if !schema_is_non_trivial(schema) {
            return None;
        }
        // Tool-level examples count: an `examples` array on the tool object.
        let tool_has_examples = tool
            .get("examples")
            .and_then(|e| e.as_array())
            .map(|a| !a.is_empty())
            .unwrap_or(false);
        if tool_has_examples {
            return None;
        }
        let any_property_has_example = schema
            .get("properties")
            .and_then(|p| p.as_object())
            .map(|props| props.values().any(property_has_example))
            .unwrap_or(false);
        if any_property_has_example {
            return None;
        }
        Some(finding(
            tool,
            self.id(),
            Severity::Warning,
            "non-trivial input schema provides no usage examples; agents handle complex parameters better with 1-5 concrete examples",
            Some("add an `examples` array to the tool or `examples`/`example`/`default` values on the parameters"),
        ))
    }
}

/// DESC-010: the description states only a vague action and never says
/// what comes back. Heuristic: pass when the description mentions a
/// return/output/result keyword or the tool declares an `outputSchema`.
/// This is intentionally coarse. It cannot judge whether the documented
/// return shape is accurate, only whether the description gestures at one
/// at all, so it stays at Warning to keep false positives cheap.
pub struct DescriptionMissingReturnFormat;
impl Rule for DescriptionMissingReturnFormat {
    fn id(&self) -> &'static str {
        "DESC-010"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let desc = description(tool);
        if desc.is_empty() {
            // Emptiness is DESC-001's job; do not double-report.
            return None;
        }
        if tool.get("outputSchema").is_some() {
            return None;
        }
        let desc_lower = desc.to_lowercase();
        let mentions_return = RETURN_KEYWORDS.iter().any(|kw| {
            if kw.contains(' ') {
                desc_lower.contains(kw)
            } else {
                desc_lower
                    .split_whitespace()
                    .any(|w| w.trim_matches(|c: char| !c.is_alphanumeric()) == *kw)
            }
        });
        if mentions_return {
            return None;
        }
        Some(finding(
            tool,
            self.id(),
            Severity::Warning,
            "description states an action but never says what the tool returns",
            Some("document the return shape, for example 'Returns a list of order objects, each with id, total, and status', or declare an outputSchema"),
        ))
    }
}

/// The boolean hint fields the MCP spec defines on a tool's `annotations`
/// object. Clients use these to present and gate a tool (a read-only tool
/// can run without confirmation; a destructive one should prompt). Each is
/// optional but, when present, must be a JSON boolean.
const ANNOTATION_HINT_FIELDS: &[&str] = &[
    "readOnlyHint",
    "destructiveHint",
    "idempotentHint",
    "openWorldHint",
];

/// DESC-011: a tool declares an `annotations` object but a known hint field
/// holds a non-boolean value. The MCP spec types every hint as an optional
/// boolean, so a string like `"yes"` or a number is malformed and a client
/// cannot read it. This is a conformance check, not a heuristic, so it fires
/// only on values that are present and the wrong JSON type.
pub struct AnnotationHintNotBoolean;
impl Rule for AnnotationHintNotBoolean {
    fn id(&self) -> &'static str {
        "DESC-011"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let annotations = tool.get("annotations")?.as_object()?;
        for hint in ANNOTATION_HINT_FIELDS {
            if let Some(value) = annotations.get(*hint) {
                if !value.is_boolean() {
                    return Some(finding(
                        tool,
                        self.id(),
                        Severity::Warning,
                        format!(
                            "annotation '{hint}' is not a boolean (found {value}); clients cannot read it"
                        ),
                        Some(&format!(
                            "set '{hint}' to a JSON boolean (true or false) or remove it"
                        )),
                    ));
                }
            }
        }
        None
    }
}

/// DESC-012: a tool declares no `annotations` object at all. Annotations
/// help a client present and gate the tool (mark it read-only, warn before a
/// destructive call), so their absence is worth flagging, but only as
/// advice. The heuristic is intentionally shallow: it cannot tell whether the
/// hints would have been accurate, only that none are declared. It also does
/// not require any specific hint, since not every tool needs every hint. That
/// is why it stays at Warning and never fires when an `annotations` object is
/// present, even an empty one.
pub struct ToolWithoutAnnotations;
impl Rule for ToolWithoutAnnotations {
    fn id(&self) -> &'static str {
        "DESC-012"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        // An object value present (even empty) signals the author considered
        // annotations; only a missing key is advisory-worthy.
        if tool.get("annotations").is_some() {
            return None;
        }
        Some(finding(
            tool,
            self.id(),
            Severity::Warning,
            "tool declares no annotations; clients cannot tell if it is read-only, destructive, or idempotent",
            Some("add an `annotations` object with the relevant hints (readOnlyHint, destructiveHint, idempotentHint, openWorldHint)"),
        ))
    }
}

/// DESC-013: a string argument whose description enumerates a fixed set of
/// allowed values but whose schema declares no `enum`. The model then has to
/// guess a valid value from prose, a common source of invalid tool calls.
/// (Inverse of DESC-007, which flags an `enum` whose values are undocumented.)
pub struct StringArgEnumerableInDescription;
impl Rule for StringArgEnumerableInDescription {
    fn id(&self) -> &'static str {
        "DESC-013"
    }
    fn check(&self, tool: &Value) -> Option<Finding> {
        let properties = tool
            .get("inputSchema")
            .and_then(|s| s.get("properties"))
            .and_then(|p| p.as_object())?;
        for (arg, prop) in properties {
            if prop.get("enum").is_some() {
                continue; // already an enum; DESC-007 covers documenting it
            }
            // Untyped properties default to string-like for this purpose.
            let is_string = prop
                .get("type")
                .and_then(|t| t.as_str())
                .map(|t| t == "string")
                .unwrap_or(true);
            if !is_string {
                continue;
            }
            let desc = prop
                .get("description")
                .and_then(|d| d.as_str())
                .unwrap_or("");
            if looks_enumerable(desc) {
                return Some(finding(
                    tool,
                    self.id(),
                    Severity::Warning,
                    format!(
                        "string argument '{arg}' lists choices in its description but declares no `enum`"
                    ),
                    Some(&format!(
                        "add an `enum` for '{arg}' so the model selects a valid value instead of guessing from prose"
                    )),
                ));
            }
        }
        None
    }
}

/// Heuristic for DESC-013: does a parameter description enumerate a fixed set
/// of values? Conservative on purpose, to avoid flagging prose that merely
/// lists examples rather than the complete allowed set.
fn looks_enumerable(desc: &str) -> bool {
    let lower = desc.to_lowercase();
    const PHRASES: [&str; 5] = [
        "one of",
        "allowed values",
        "valid values",
        "options are",
        "either ",
    ];
    PHRASES.iter().any(|p| lower.contains(p)) || desc.contains(" | ")
}

#[cfg(test)]
mod tests {
    use super::*;
    use serde_json::json;

    fn make_tool(name: &str, desc: &str, schema: Value) -> Value {
        json!({ "name": name, "description": desc, "inputSchema": schema })
    }

    fn collect_findings(tool: &Value, rule: &dyn Rule) -> Vec<Finding> {
        rule.check(tool).into_iter().collect()
    }

    #[test]
    fn too_short_triggers_critical() {
        let tool = make_tool("get_x", "short", json!({}));
        let f = collect_findings(&tool, &DescriptionTooShort);
        assert_eq!(f.len(), 1);
        assert_eq!(f[0].severity, Severity::Critical);
    }

    #[test]
    fn too_short_passes_at_20_chars() {
        let tool = make_tool("get_x", "exactly twenty chars", json!({}));
        let f = collect_findings(&tool, &DescriptionTooShort);
        assert!(f.is_empty());
    }

    #[test]
    fn too_long_triggers_warning() {
        let long = "abc ".repeat(200);
        let tool = make_tool("verbose_tool", &long, json!({}));
        let f = collect_findings(&tool, &DescriptionTooLong);
        assert_eq!(f.len(), 1);
        assert_eq!(f[0].severity, Severity::Warning);
    }

    #[test]
    fn duplicates_name_triggers_critical() {
        let tool = make_tool("delete_issue", "delete_issue", json!({}));
        let f = collect_findings(&tool, &DescriptionDuplicatesName);
        assert_eq!(f.len(), 1);
        assert_eq!(f[0].severity, Severity::Critical);
    }

    #[test]
    fn missing_verb_triggers_warning() {
        let tool = make_tool("greet", "a friendly entity for the user", json!({}));
        let f = collect_findings(&tool, &DescriptionMissingVerb);
        assert_eq!(f.len(), 1);
    }

    #[test]
    fn verb_present_passes() {
        let tool = make_tool("greet", "returns a polite greeting for the user", json!({}));
        let f = collect_findings(&tool, &DescriptionMissingVerb);
        assert!(f.is_empty());
    }

    #[test]
    fn positional_phrase_triggers_warning() {
        let tool = make_tool(
            "get_x",
            "see also the previous tool for related state",
            json!({}),
        );
        let f = collect_findings(&tool, &DescriptionReferencesPosition);
        assert_eq!(f.len(), 1);
    }

    #[test]
    fn missing_arg_description_triggers_warning() {
        let tool = make_tool(
            "create_x",
            "creates a thing with a title",
            json!({
                "type": "object",
                "required": ["title"],
                "properties": { "title": { "type": "string" } }
            }),
        );
        let f = collect_findings(&tool, &ArgumentMissingDescription);
        assert_eq!(f.len(), 1);
    }

    #[test]
    fn enum_without_value_descriptions_triggers_warning() {
        let tool = make_tool(
            "summarize",
            "summarizes an issue in some style",
            json!({
                "properties": {
                    "style": { "type": "string", "enum": ["bullet", "paragraph"], "description": "the output format" }
                }
            }),
        );
        let f = collect_findings(&tool, &EnumArgWithoutValueDescriptions);
        assert_eq!(f.len(), 1);
    }

    #[test]
    fn arg_description_longer_than_tool_triggers_warning() {
        let tool = make_tool(
            "x",
            "create",
            json!({
                "properties": {
                    "title": { "type": "string", "description": "this description is intentionally long and goes on for many many characters to overflow the tool description for testing purposes" }
                }
            }),
        );
        let f = collect_findings(&tool, &ArgDescriptionLongerThanTool);
        assert_eq!(f.len(), 1);
    }

    #[test]
    fn non_trivial_schema_without_examples_triggers_warning() {
        let tool = make_tool(
            "create_order",
            "creates an order for a customer",
            json!({
                "type": "object",
                "required": ["customer_id", "amount"],
                "properties": {
                    "customer_id": { "type": "string", "description": "the customer id" },
                    "amount": { "type": "number", "description": "the order total in cents" }
                }
            }),
        );
        let f = collect_findings(&tool, &NonTrivialSchemaWithoutExamples);
        assert_eq!(f.len(), 1);
        assert_eq!(f[0].severity, Severity::Warning);
        assert_eq!(f[0].rule_id, "DESC-009");
    }

    #[test]
    fn non_trivial_schema_with_property_examples_passes() {
        let tool = make_tool(
            "create_order",
            "creates an order for a customer",
            json!({
                "type": "object",
                "required": ["customer_id", "amount"],
                "properties": {
                    "customer_id": { "type": "string", "description": "the customer id", "examples": ["cus_123"] },
                    "amount": { "type": "number", "description": "the order total in cents", "default": 100 }
                }
            }),
        );
        let f = collect_findings(&tool, &NonTrivialSchemaWithoutExamples);
        assert!(f.is_empty());
    }

    #[test]
    fn trivial_single_string_schema_is_exempt_from_examples_rule() {
        let tool = make_tool(
            "echo",
            "returns the supplied message verbatim",
            json!({
                "type": "object",
                "properties": {
                    "message": { "type": "string", "description": "text to echo" }
                }
            }),
        );
        let f = collect_findings(&tool, &NonTrivialSchemaWithoutExamples);
        assert!(f.is_empty());
    }

    #[test]
    fn missing_return_format_triggers_warning() {
        let tool = make_tool("search_orders", "Run the order search", json!({}));
        let f = collect_findings(&tool, &DescriptionMissingReturnFormat);
        assert_eq!(f.len(), 1);
        assert_eq!(f[0].severity, Severity::Warning);
        assert_eq!(f[0].rule_id, "DESC-010");
    }

    #[test]
    fn description_documenting_return_shape_passes() {
        let tool = make_tool(
            "search_orders",
            "Searches orders and returns a list of order objects, each with id, total, and status",
            json!({}),
        );
        let f = collect_findings(&tool, &DescriptionMissingReturnFormat);
        assert!(f.is_empty());
    }

    #[test]
    fn output_schema_satisfies_return_format_rule() {
        let mut tool = make_tool("query_orders", "Execute order query", json!({}));
        tool["outputSchema"] = json!({ "type": "array" });
        let f = collect_findings(&tool, &DescriptionMissingReturnFormat);
        assert!(f.is_empty());
    }

    #[test]
    fn non_boolean_annotation_hint_triggers_warning() {
        let mut tool = make_tool("delete_file", "Deletes a file at the given path", json!({}));
        tool["annotations"] = json!({ "destructiveHint": "yes" });
        let f = collect_findings(&tool, &AnnotationHintNotBoolean);
        assert_eq!(f.len(), 1);
        assert_eq!(f[0].severity, Severity::Warning);
        assert_eq!(f[0].rule_id, "DESC-011");
    }

    #[test]
    fn well_formed_boolean_annotations_pass() {
        let mut tool = make_tool(
            "read_file",
            "Reads a file and returns its contents",
            json!({}),
        );
        tool["annotations"] = json!({
            "readOnlyHint": true,
            "destructiveHint": false,
            "idempotentHint": true,
            "openWorldHint": false
        });
        let f = collect_findings(&tool, &AnnotationHintNotBoolean);
        assert!(f.is_empty());
    }

    #[test]
    fn missing_annotations_object_does_not_trigger_malformed_rule() {
        let tool = make_tool(
            "read_file",
            "Reads a file and returns its contents",
            json!({}),
        );
        let f = collect_findings(&tool, &AnnotationHintNotBoolean);
        assert!(f.is_empty());
    }

    #[test]
    fn tool_without_annotations_triggers_warning() {
        let tool = make_tool("delete_file", "Deletes a file at the given path", json!({}));
        let f = collect_findings(&tool, &ToolWithoutAnnotations);
        assert_eq!(f.len(), 1);
        assert_eq!(f[0].severity, Severity::Warning);
        assert_eq!(f[0].rule_id, "DESC-012");
    }

    #[test]
    fn tool_with_annotations_object_passes_absence_rule() {
        let mut tool = make_tool(
            "read_file",
            "Reads a file and returns its contents",
            json!({}),
        );
        tool["annotations"] = json!({ "readOnlyHint": true });
        let f = collect_findings(&tool, &ToolWithoutAnnotations);
        assert!(f.is_empty());
    }

    #[test]
    fn empty_annotations_object_passes_absence_rule() {
        let mut tool = make_tool(
            "read_file",
            "Reads a file and returns its contents",
            json!({}),
        );
        tool["annotations"] = json!({});
        let f = collect_findings(&tool, &ToolWithoutAnnotations);
        assert!(f.is_empty());
    }

    #[test]
    fn lint_catalog_returns_findings_grouped_by_tool() {
        let mut clean = make_tool("greet", "returns a polite greeting for the user", json!({}));
        // Carry well-formed annotations so DESC-011 and DESC-012 also pass and
        // this tool stays clean, exercising the synthetic Pass path.
        clean["annotations"] = json!({ "readOnlyHint": true });
        let catalog = json!({
            "tools": [
                make_tool("x", "tiny", json!({})),
                clean
            ]
        });
        let rules = default_rules();
        let rule_refs: Vec<&dyn Rule> = rules.iter().map(|r| r.as_ref()).collect();
        let report = lint_catalog(&catalog, &rule_refs);
        assert_eq!(report.tools_checked, 2);
        assert!(report.has_critical());
        assert!(report.pass_count() >= 1);
    }

    #[test]
    fn lint_catalog_handles_empty_catalog() {
        let report = lint_catalog(&json!({"tools": []}), &[]);
        assert_eq!(report.tools_checked, 0);
        assert!(report.findings.is_empty());
    }

    #[test]
    fn render_pretty_includes_severity_markers_and_summary() {
        let report = LintReport {
            findings: vec![Finding {
                tool_name: "greet".into(),
                rule_id: "DESC-001",
                severity: Severity::Critical,
                message: "too short".into(),
                suggestion: Some("expand".into()),
            }],
            tools_checked: 1,
        };
        let mut buf = Vec::new();
        render_pretty(&report, &mut buf).unwrap();
        let s = String::from_utf8(buf).unwrap();
        assert!(s.contains("[XX]"));
        assert!(s.contains("Quality summary"));
        assert!(s.contains("1 critical"));
    }

    #[test]
    fn string_arg_listing_choices_without_enum_triggers_warning() {
        let tool = make_tool(
            "set_status",
            "Set the order status for a record",
            json!({
                "type": "object",
                "properties": {
                    "status": {
                        "type": "string",
                        "description": "The new status, one of open, closed, or pending"
                    }
                }
            }),
        );
        let f = collect_findings(&tool, &StringArgEnumerableInDescription);
        assert_eq!(f.len(), 1);
        assert_eq!(f[0].rule_id, "DESC-013");
        assert_eq!(f[0].severity, Severity::Warning);
    }

    #[test]
    fn string_arg_that_already_declares_enum_is_not_flagged() {
        let tool = make_tool(
            "set_status",
            "Set the order status for a record",
            json!({
                "type": "object",
                "properties": {
                    "status": {
                        "type": "string",
                        "enum": ["open", "closed"],
                        "description": "The new status, one of open or closed"
                    }
                }
            }),
        );
        let f = collect_findings(&tool, &StringArgEnumerableInDescription);
        assert!(f.is_empty());
    }

    #[test]
    fn free_text_string_arg_without_choice_language_passes() {
        let tool = make_tool(
            "search",
            "Search records by free text",
            json!({
                "type": "object",
                "properties": {
                    "query": { "type": "string", "description": "Free-text search query" }
                }
            }),
        );
        let f = collect_findings(&tool, &StringArgEnumerableInDescription);
        assert!(f.is_empty());
    }
}
