ryo_suggest/

suggest.rs

//! Suggest trait and core types for continuous refactoring
//!
//! This module defines the domain model for detecting improvement opportunities
//! and generating MutationSpecs for execution.
//!
//! # Architecture: Suggest and the Two-Layer Mutation System
//!
//! Ryo has two mutation specification layers:
//! - [`Intent`](ryo_app::intent::Intent): Public DSL for CLI users (pattern-based)
//! - [`MutationSpec`]: Execution-level specification (concrete targets)
//!
//! The `Suggest` system **bypasses Intent** and generates `MutationSpec` directly:
//!
//! ```text
//! ┌─────────────────────────────────────────────────────────────────┐
//! │  Suggest::detect()                                              │
//! │  - Analyzes code for improvement opportunities                  │
//! │  - Returns SuggestOpportunity with concrete SymbolIds           │
//! └───────────────────────────┬─────────────────────────────────────┘
//!                             ↓
//! ┌─────────────────────────────────────────────────────────────────┐
//! │  Suggest::to_mutation_specs()                                   │
//! │  - Converts opportunity to MutationSpec(s)                      │
//! │  - Skips Intent layer (no pattern resolution needed)            │
//! └───────────────────────────┬─────────────────────────────────────┘
//!                             ↓
//! ┌─────────────────────────────────────────────────────────────────┐
//! │  Executor                                                       │
//! │  - Applies MutationSpecs to AST                                 │
//! └─────────────────────────────────────────────────────────────────┘
//! ```
//!
//! ## Why Bypass Intent?
//!
//! - Suggest already has resolved symbols from `AnalysisContext`
//! - No need for pattern matching (targets are concrete)
//! - More efficient: direct path to execution
//!
//! ## Re-export of MutationSpec
//!
//! `MutationSpec` is re-exported here for convenience. Users of the Suggest
//! system can import both `Suggest` trait and `MutationSpec` from this module
//! without needing to depend on `ryo-executor` directly.

use std::collections::HashMap;
use std::fmt;

use ryo_analysis::context::AnalysisContext;
use ryo_analysis::{SymbolId, SymbolPath};
use serde::{Deserialize, Serialize};
use thiserror::Error;

// =============================================================================
// Error Types
// =============================================================================

/// Error type for `to_mutation_specs()` operations
#[derive(Debug, Error)]
pub enum SuggestError {
    #[error("Failed to resolve module path for spec at {path}")]
    ModulePathResolution { path: String },

    #[error("Invalid symbol path: {reason}")]
    InvalidSymbolPath { reason: String },

    #[error("Missing required context: {context}")]
    MissingContext { context: String },
}

/// Result type for Suggest operations
pub type SuggestResult<T> = Result<T, SuggestError>;

// =============================================================================
// Parameterized Suggest Types
// =============================================================================

/// Parameters for parameterized suggestions.
///
/// Used to pass variables like `{ "name": "Order" }` to generate
/// context-specific code (e.g., `OrderAPI`, `OrderService`).
pub type SuggestParams = HashMap<String, String>;

/// Definition of a parameter for parameterized suggestions.
///
/// Used by LLMs to understand what parameters a suggestion accepts.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct ParamDef {
    /// Parameter name (e.g., "name")
    pub name: String,
    /// Human-readable description (e.g., "Name of the API to generate")
    pub description: String,
    /// Whether this parameter is required
    pub required: bool,
}

impl ParamDef {
    /// Create a required parameter definition
    pub fn required(name: impl Into<String>, description: impl Into<String>) -> Self {
        Self {
            name: name.into(),
            description: description.into(),
            required: true,
        }
    }

    /// Create an optional parameter definition
    pub fn optional(name: impl Into<String>, description: impl Into<String>) -> Self {
        Self {
            name: name.into(),
            description: description.into(),
            required: false,
        }
    }
}

// Re-export MutationSpec and related types from ryo-executor for convenience.
// Suggest implementations generate MutationSpecs directly, bypassing Intent.
pub use ryo_executor::executor::{EnumToTraitStrategy, MatchHandling, MutationSpec};

/// Category for suggestion classification
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub enum SuggestCategory {
    /// Derive macros (Default, Clone, Debug, etc.)
    Derive,
    /// Design patterns (Builder, Factory, etc.)
    Pattern,
    /// Performance improvements (Atomic, RwLock, etc.)
    Performance,
    /// Safety improvements (LockScope, etc.)
    Safety,
    /// Idiomatic transformations (match→if-let, etc.)
    Idiom,
    /// Code organization (extract, inline, etc.)
    Refactor,
    /// Lint rules (code quality checks, test coverage, etc.)
    Lint,
}

impl fmt::Display for SuggestCategory {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            Self::Derive => write!(f, "Derive"),
            Self::Pattern => write!(f, "Pattern"),
            Self::Performance => write!(f, "Performance"),
            Self::Safety => write!(f, "Safety"),
            Self::Idiom => write!(f, "Idiom"),
            Self::Refactor => write!(f, "Refactor"),
            Self::Lint => write!(f, "Lint"),
        }
    }
}

/// Safety classification for auto-application decisions
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
pub enum SafetyLevel {
    /// Fully automatic - no side effects, guaranteed safe
    /// Examples: Add #[derive(Default)] to struct with all-defaultable fields
    Auto,

    /// Confirmation recommended - standard refactoring
    /// Examples: Generate Builder pattern, extract function
    Confirm,

    /// Manual only - potential breaking changes
    /// Examples: Change public API, remove unused code
    Manual,
}

impl fmt::Display for SafetyLevel {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            Self::Auto => write!(f, "AUTO"),
            Self::Confirm => write!(f, "CONFIRM"),
            Self::Manual => write!(f, "MANUAL"),
        }
    }
}

// =============================================================================
// Priority Computation
// =============================================================================

/// Compute priority score from confidence, safety level, and pattern weight.
///
/// Returns 0-255 (higher = more important).
///
/// # Formula
/// ```text
/// priority = confidence × safety_weight × normalized_pattern_weight × 255
/// ```
///
/// Where:
/// - `confidence`: 0.0-1.0 from the opportunity
/// - `safety_weight`: Auto=1.0, Confirm=0.7, Manual=0.4
/// - `normalized_pattern_weight`: pattern_weight normalized to 0.4-1.0 range
///   (input range 0.5-2.5 maps to 0.4-1.0)
///
/// # Examples
/// ```ignore
/// // High priority: high confidence + auto safety + critical pattern
/// compute_priority(0.95, SafetyLevel::Auto, 2.5) // → ~242
///
/// // Low priority: medium confidence + manual safety + low pattern weight
/// compute_priority(0.7, SafetyLevel::Manual, 0.8) // → ~29
/// ```
pub fn compute_priority(confidence: f32, safety: SafetyLevel, pattern_weight: f32) -> u8 {
    let safety_weight = match safety {
        SafetyLevel::Auto => 1.0,
        SafetyLevel::Confirm => 0.7,
        SafetyLevel::Manual => 0.4,
    };
    // Normalize pattern_weight: 0.5-2.5 → 0.4-1.0
    // This ensures pattern weight affects but doesn't dominate priority
    let normalized_pattern = (pattern_weight / 2.5).clamp(0.4, 1.0);
    (confidence * safety_weight * normalized_pattern * 255.0).clamp(0.0, 255.0) as u8
}

/// Newtype for opportunity IDs (unique within a detection run)
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub struct OpportunityId(pub u32);

impl OpportunityId {
    pub fn new(id: u32) -> Self {
        Self(id)
    }

    pub fn as_u32(self) -> u32 {
        self.0
    }
}

impl fmt::Display for OpportunityId {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "O{:04}", self.0)
    }
}

/// Location information for a suggestion
///
/// Uses SymbolId/SymbolPath for precise symbol reference.
/// The `file` field is derived from the symbol's span when available.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct SuggestLocation {
    /// Symbol ID for precise reference
    pub symbol_id: SymbolId,

    /// Symbol path (e.g., "test_crate::module::SymbolName")
    pub symbol_path: SymbolPath,

    /// File path (derived from symbol span, for display)
    pub file: String,
}

impl SuggestLocation {
    /// Create a new SuggestLocation from symbol information
    pub fn new(symbol_id: SymbolId, symbol_path: SymbolPath, file: impl Into<String>) -> Self {
        Self {
            symbol_id,
            symbol_path,
            file: file.into(),
        }
    }

    /// Create from AnalysisContext by looking up the symbol
    pub fn from_context(ctx: &AnalysisContext, symbol_id: SymbolId) -> Option<Self> {
        let symbol_path = ctx.registry().resolve(symbol_id)?;
        let file = ctx
            .registry()
            .span(symbol_id)
            .map(|span| span.file.to_string())
            .unwrap_or_else(|| symbol_path.crate_name().to_string());
        Some(Self::new(symbol_id, symbol_path.clone(), file))
    }

    /// Get the symbol name (last component of path)
    pub fn symbol_name(&self) -> &str {
        self.symbol_path.name()
    }

    /// Create a test location with dummy SymbolId
    #[cfg(any(test, feature = "testing"))]
    pub fn for_test(file: impl Into<String>, symbol_name: impl Into<String>) -> Self {
        let name = symbol_name.into();
        // Use a fixed test SymbolId (0v1 format)
        let symbol_id = SymbolId::parse("0v1").expect("test SymbolId");
        let symbol_path = SymbolPath::builder("test")
            .push(&name)
            .build()
            .expect("test path");
        Self {
            symbol_id,
            symbol_path,
            file: file.into(),
        }
    }
}

impl fmt::Display for SuggestLocation {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        write!(f, "{} ({})", self.symbol_path, self.file)
    }
}

/// Severity level for lint violations (maps to SafetyLevel)
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum LintSeverity {
    /// Informational message
    Info,
    /// Warning that should be addressed
    Warning,
    /// Error that must be fixed
    Error,
}

impl fmt::Display for LintSeverity {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            Self::Info => write!(f, "info"),
            Self::Warning => write!(f, "warning"),
            Self::Error => write!(f, "error"),
        }
    }
}

impl std::str::FromStr for LintSeverity {
    type Err = String;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s.to_lowercase().as_str() {
            "info" | "hint" => Ok(Self::Info),
            "warning" | "warn" => Ok(Self::Warning),
            "error" | "err" => Ok(Self::Error),
            _ => Err(format!("Unknown severity: {}", s)),
        }
    }
}

impl From<LintSeverity> for SafetyLevel {
    fn from(severity: LintSeverity) -> Self {
        match severity {
            LintSeverity::Info => SafetyLevel::Auto,
            LintSeverity::Warning => SafetyLevel::Confirm,
            LintSeverity::Error => SafetyLevel::Manual,
        }
    }
}

impl From<SafetyLevel> for LintSeverity {
    fn from(level: SafetyLevel) -> Self {
        match level {
            SafetyLevel::Auto => LintSeverity::Info,
            SafetyLevel::Confirm => LintSeverity::Warning,
            SafetyLevel::Manual => LintSeverity::Error,
        }
    }
}

/// Type-safe context for different suggestion types
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(tag = "type")]
pub enum OpportunityContext {
    /// Default derive opportunity
    Derive {
        derive_name: String,
        #[serde(default)]
        missing_impls: Vec<String>,
    },

    /// Builder pattern opportunity
    Builder {
        struct_name: String,
        field_count: usize,
        has_required_fields: bool,
    },

    /// Atomic replacement opportunity
    Atomic {
        current_type: String,
        suggested_atomic: String,
    },

    /// From/Into implementation opportunity
    FromInto {
        source_type: String,
        target_type: String,
    },

    /// Lint violation context
    Lint {
        /// Rule code (e.g., "RL001")
        code: String,
        /// Rule name (e.g., "require-test-for-mutation")
        rule: String,
        /// Severity level
        severity: LintSeverity,
        /// Optional suggestion for fixing
        #[serde(default)]
        suggestion: Option<String>,
        /// Expected pattern (what should exist)
        #[serde(default)]
        expected: Option<String>,
        /// Actual pattern (what was found)
        #[serde(default)]
        actual: Option<String>,
    },

    /// Spec (domain specification) opportunity
    Spec {
        /// Rule code (e.g., "RS001")
        code: String,
        /// Spec alias name (e.g., "TaskSpec")
        #[serde(default)]
        alias_name: Option<String>,
        /// Base type name (e.g., "Task")
        #[serde(default)]
        base_type: Option<String>,
        /// Group name (e.g., "DomainGroup")
        #[serde(default)]
        group: Option<String>,
        /// Related types (for relation suggestions)
        #[serde(default)]
        related_types: Vec<String>,
        /// Optional suggestion for fixing
        #[serde(default)]
        suggestion: Option<String>,
    },

    /// Generic context for extensibility
    Custom {
        #[serde(flatten)]
        data: serde_json::Value,
    },

    /// Parameterized code generation context.
    ///
    /// Used when generating code from a pattern with user-provided parameters.
    /// Example: API pattern with `{ "name": "Order" }` generates `OrderAPI`.
    Generation {
        /// Pattern name (e.g., "api", "domain", "service")
        pattern: String,
        /// User-provided parameters (e.g., `{ "name": "Order" }`)
        params: HashMap<String, String>,
    },
}

// =============================================================================
// Symbol Scope
// =============================================================================

/// The code scope where a symbol is defined.
///
/// Used to classify suggestions by their origin context, enabling
/// consumers to filter suggestions (e.g., show only library code issues).
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, Default)]
pub enum SymbolScope {
    /// Library code (src/lib.rs and its modules)
    #[default]
    Lib,
    /// Binary crate code (src/main.rs, src/bin/*.rs and their modules)
    Bin,
    /// Test code (#[cfg(test)] modules, tests/ directory)
    Test,
}

impl SymbolScope {
    /// Check if this scope represents production code (Lib or Bin)
    pub fn is_production(&self) -> bool {
        matches!(self, Self::Lib | Self::Bin)
    }
}

impl fmt::Display for SymbolScope {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            Self::Lib => write!(f, "lib"),
            Self::Bin => write!(f, "bin"),
            Self::Test => write!(f, "test"),
        }
    }
}

impl std::str::FromStr for SymbolScope {
    type Err = String;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s.to_lowercase().as_str() {
            "lib" => Ok(Self::Lib),
            "bin" => Ok(Self::Bin),
            "test" => Ok(Self::Test),
            other => Err(format!(
                "unknown scope: '{}' (expected: lib, bin, test)",
                other
            )),
        }
    }
}

impl SymbolScope {
    /// Resolve the scope for a symbol from AnalysisContext.
    ///
    /// Resolution order:
    /// 1. Check if symbol path contains a "tests" segment → Test
    /// 2. Check if symbol's file path is in tests/ directory → Test
    /// 3. Check if symbol's crate has a binary entry point → Bin
    /// 4. Otherwise → Lib
    ///
    /// The `binary_crates` set should be pre-computed via
    /// `SymbolScope::binary_crate_names(ctx)` for efficiency.
    pub fn resolve(
        ctx: &AnalysisContext,
        symbol_id: SymbolId,
        binary_crates: &std::collections::HashSet<String>,
    ) -> Self {
        // 1. Check symbol path for test module segments.
        //    Matches:
        //      - "tests" / "tests_*"  — any segment (standard #[cfg(test)] names)
        //      - "test_*"             — non-root segments only (module names like
        //        "test_utils", "test_harness"; excludes crate names like "test_crate")
        if let Some(path) = ctx.registry.path(symbol_id) {
            for (i, segment) in path.segments().enumerate() {
                if segment == "tests" || segment.starts_with("tests_") {
                    return Self::Test;
                }
                // test_ prefix only for module segments (skip crate name at index 0)
                if i > 0 && segment.starts_with("test_") {
                    return Self::Test;
                }
            }
        }

        // 2. Check file path for tests/ directory
        if let Some(span) = ctx.registry.span(symbol_id) {
            let relative = span.file.as_relative().to_string_lossy();
            if relative.contains("/tests/") || relative.starts_with("tests/") {
                return Self::Test;
            }

            // 3. Check if this symbol's crate is a binary crate
            let crate_name = span.file.crate_name().as_str();
            if binary_crates.contains(crate_name) {
                return Self::Bin;
            }
        }

        // 4. Default: library code
        Self::Lib
    }

    /// Pre-compute the set of binary crate names from AnalysisContext.
    ///
    /// A crate is binary if any of its files is a binary entry point
    /// (main.rs or src/bin/*.rs).
    pub fn binary_crate_names(ctx: &AnalysisContext) -> std::collections::HashSet<String> {
        ctx.files()
            .keys()
            .filter(|f| f.is_binary_entry())
            .map(|f| f.crate_name().as_str().to_string())
            .collect()
    }
}

// =============================================================================
// Suggest Opportunity
// =============================================================================

/// A detected opportunity for improvement
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct SuggestOpportunity {
    /// Unique ID within this detection run
    pub id: OpportunityId,

    /// Target symbol(s) for this opportunity
    pub targets: Vec<SymbolId>,

    /// Primary location for display
    pub location: SuggestLocation,

    /// Human-readable suggestion message
    pub message: String,

    /// Confidence score (0.0 - 1.0)
    pub confidence: f32,

    /// Pattern-specific context (type-safe via enum)
    pub context: OpportunityContext,

    /// Code scope where this opportunity was detected
    #[serde(default)]
    pub scope: SymbolScope,
}

impl SuggestOpportunity {
    /// Create a new opportunity
    ///
    /// Scope defaults to `Lib`. The pipeline (detect_with_config) resolves
    /// and overwrites the scope based on symbol context.
    pub fn new(
        id: OpportunityId,
        targets: Vec<SymbolId>,
        location: SuggestLocation,
        message: impl Into<String>,
        confidence: f32,
        context: OpportunityContext,
    ) -> Self {
        Self {
            id,
            targets,
            location,
            message: message.into(),
            confidence: confidence.clamp(0.0, 1.0),
            context,
            scope: SymbolScope::default(),
        }
    }

    /// Set the scope for this opportunity
    pub fn with_scope(mut self, scope: SymbolScope) -> Self {
        self.scope = scope;
        self
    }

    /// Get the primary target symbol (first in the list)
    pub fn primary_target(&self) -> Option<SymbolId> {
        self.targets.first().copied()
    }

    /// Override severity for Lint context.
    ///
    /// Only affects Lint context - other contexts are unchanged.
    /// Returns self for chaining.
    pub fn with_severity_override(mut self, new_severity: LintSeverity) -> Self {
        if let OpportunityContext::Lint {
            ref mut severity, ..
        } = self.context
        {
            *severity = new_severity;
        }
        self
    }

    /// Get the lint severity if this is a Lint context
    pub fn lint_severity(&self) -> Option<LintSeverity> {
        if let OpportunityContext::Lint { severity, .. } = &self.context {
            Some(*severity)
        } else {
            None
        }
    }

    /// Get the lint rule code if this is a Lint context
    pub fn lint_code(&self) -> Option<&str> {
        if let OpportunityContext::Lint { code, .. } = &self.context {
            Some(code)
        } else {
            None
        }
    }

    /// Get the spec rule code if this is a Spec context
    pub fn spec_code(&self) -> Option<&str> {
        if let OpportunityContext::Spec { code, .. } = &self.context {
            Some(code)
        } else {
            None
        }
    }

    // ========== Builder Methods ==========

    /// Set suggestion text for Lint or Spec context.
    /// Returns self for chaining.
    pub fn with_suggestion(mut self, suggestion: impl Into<String>) -> Self {
        match &mut self.context {
            OpportunityContext::Lint {
                suggestion: ref mut s,
                ..
            } => {
                *s = Some(suggestion.into());
            }
            OpportunityContext::Spec {
                suggestion: ref mut s,
                ..
            } => {
                *s = Some(suggestion.into());
            }
            _ => {}
        }
        self
    }

    /// Set expected/actual for Lint context.
    /// Returns self for chaining.
    pub fn with_expected_actual(
        mut self,
        expected: impl Into<String>,
        actual: impl Into<String>,
    ) -> Self {
        if let OpportunityContext::Lint {
            expected: ref mut e,
            actual: ref mut a,
            ..
        } = &mut self.context
        {
            *e = Some(expected.into());
            *a = Some(actual.into());
        }
        self
    }

    /// Set related types for Spec context.
    /// Returns self for chaining.
    pub fn with_related_types(mut self, types: Vec<String>) -> Self {
        if let OpportunityContext::Spec {
            related_types: ref mut rt,
            ..
        } = &mut self.context
        {
            *rt = types;
        }
        self
    }

    /// Override confidence score.
    /// Returns self for chaining.
    pub fn with_confidence(mut self, confidence: f32) -> Self {
        self.confidence = confidence.clamp(0.0, 1.0);
        self
    }
}

/// Detection + MutationSpec generation capability
///
/// Each implementor:
/// 1. Detects opportunities from AnalysisContext for given symbols
/// 2. Generates MutationSpecs (NOT Mutations) for each opportunity
///
/// This separation ensures:
/// - Detect logic stays with Suggest implementations
/// - Execution flows through standard MutationSpec → Executor pipeline
/// - No special handling needed in Executor for Suggest-originated mutations
///
/// # Parameterized Suggestions
///
/// Suggestions can accept external parameters for code generation.
/// Override `accepts_params()`, `param_schema()`, and `detect_with_params()`
/// to enable parameterized generation.
///
/// ```ignore
/// impl Suggest for ApiPatternSuggest {
///     fn accepts_params(&self) -> bool { true }
///
///     fn param_schema(&self) -> Vec<ParamDef> {
///         vec![ParamDef::required("name", "API name (e.g., Order)")]
///     }
///
///     fn detect_with_params(&self, ctx, symbols, params) -> Vec<SuggestOpportunity> {
///         let name = params.get("name").unwrap();
///         // Generate OrderAPI opportunities...
///     }
/// }
/// ```
pub trait Suggest: Send + Sync {
    /// Name of this suggestion pattern (e.g., "Builder", "Default")
    fn name(&self) -> &'static str;

    /// Human-readable description
    fn description(&self) -> &str;

    /// Category for filtering/grouping
    fn category(&self) -> SuggestCategory;

    /// Safety level for auto-application decisions
    fn safety_level(&self) -> SafetyLevel;

    /// Priority weight for ranking (higher = more important)
    fn priority_weight(&self) -> f32 {
        1.0
    }

    /// Optional rule ID for pattern-based rules (e.g., "RL021").
    /// Returns None for non-pattern suggestions.
    fn rule_id(&self) -> Option<&str> {
        None
    }

    /// Target scopes where this suggest applies.
    ///
    /// When non-empty, opportunities are filtered to only those in the specified scopes.
    /// When empty (default), the suggest applies to all scopes.
    ///
    /// Example: `vec![SymbolScope::Lib, SymbolScope::Bin]` excludes test code.
    fn target_scopes(&self) -> Vec<SymbolScope> {
        vec![]
    }

    // ========== Parameterized Suggest Support ==========

    /// Whether this suggestion accepts external parameters.
    ///
    /// If true, `detect_with_params` should be overridden to handle parameters.
    /// LLMs can query this to determine if a suggestion supports generation mode.
    fn accepts_params(&self) -> bool {
        false
    }

    /// Schema of accepted parameters (for LLM consumption).
    ///
    /// Returns parameter definitions that LLMs can use to construct
    /// valid parameter sets for `detect_with_params`.
    fn param_schema(&self) -> Vec<ParamDef> {
        vec![]
    }

    /// Detect opportunities with external parameters.
    ///
    /// Override this method for parameterized code generation.
    /// The `params` map contains user-provided values like `{ "name": "Order" }`.
    ///
    /// Default implementation ignores params and delegates to `detect`.
    fn detect_with_params(
        &self,
        ctx: &AnalysisContext,
        symbols: &[SymbolId],
        _params: &SuggestParams,
    ) -> Vec<SuggestOpportunity> {
        self.detect(ctx, symbols)
    }

    // ========== Core Detection ==========

    /// Detect opportunities for the given symbols
    ///
    /// Takes:
    /// - ctx: Full AnalysisContext for graph queries, type info, etc.
    /// - symbols: Target symbols to check (batch processing)
    ///
    /// Returns: Opportunities found (may be empty)
    fn detect(&self, ctx: &AnalysisContext, symbols: &[SymbolId]) -> Vec<SuggestOpportunity>;

    /// Convert a detected opportunity to executable MutationSpecs
    ///
    /// Takes:
    /// - ctx: AnalysisContext for resolving SymbolPath, type info, etc.
    /// - opportunity: The detected opportunity
    ///
    /// Returns Vec because one opportunity may require multiple specs
    /// (e.g., Builder pattern needs struct + impl + methods)
    ///
    /// # Errors
    /// Returns `SuggestError` if mutation spec generation fails
    fn to_mutation_specs(
        &self,
        ctx: &AnalysisContext,
        opportunity: &SuggestOpportunity,
    ) -> SuggestResult<Vec<MutationSpec>>;
}

/// Boxed Suggest trait object
pub type SuggestBox = Box<dyn Suggest>;

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

    #[test]
    fn test_safety_level_ordering() {
        assert!(SafetyLevel::Auto < SafetyLevel::Confirm);
        assert!(SafetyLevel::Confirm < SafetyLevel::Manual);
    }

    #[test]
    fn test_opportunity_id_display() {
        let id = OpportunityId::new(42);
        assert_eq!(id.to_string(), "O0042");
    }

    #[test]
    fn test_suggest_location_display() {
        let loc = SuggestLocation::for_test("src/lib.rs", "MyStruct");
        // for_test creates path as "test::MyStruct"
        assert_eq!(loc.to_string(), "test::MyStruct (src/lib.rs)");
    }

    #[test]
    fn test_opportunity_context_serde() {
        let ctx = OpportunityContext::Builder {
            struct_name: "Config".into(),
            field_count: 5,
            has_required_fields: true,
        };

        let json = serde_json::to_string(&ctx).unwrap();
        let parsed: OpportunityContext = serde_json::from_str(&json).unwrap();

        match parsed {
            OpportunityContext::Builder {
                struct_name,
                field_count,
                ..
            } => {
                assert_eq!(struct_name, "Config");
                assert_eq!(field_count, 5);
            }
            _ => panic!("Wrong variant"),
        }
    }

    #[test]
    fn test_confidence_clamping() {
        let opp = SuggestOpportunity::new(
            OpportunityId::new(1),
            vec![],
            SuggestLocation::for_test("test.rs", "Test"),
            "Test message",
            1.5, // Over 1.0
            OpportunityContext::Derive {
                derive_name: "Default".into(),
                missing_impls: vec![],
            },
        );
        assert_eq!(opp.confidence, 1.0);

        let opp2 = SuggestOpportunity::new(
            OpportunityId::new(2),
            vec![],
            SuggestLocation::for_test("test.rs", "Test"),
            "Test message",
            -0.5, // Below 0.0
            OpportunityContext::Derive {
                derive_name: "Default".into(),
                missing_impls: vec![],
            },
        );
        assert_eq!(opp2.confidence, 0.0);
    }

    #[test]
    fn test_lint_severity_from_str() {
        use std::str::FromStr;

        // Lowercase
        assert_eq!(LintSeverity::from_str("info").unwrap(), LintSeverity::Info);
        assert_eq!(
            LintSeverity::from_str("warning").unwrap(),
            LintSeverity::Warning
        );
        assert_eq!(
            LintSeverity::from_str("error").unwrap(),
            LintSeverity::Error
        );

        // Titlecase (from YAML/TOML)
        assert_eq!(LintSeverity::from_str("Info").unwrap(), LintSeverity::Info);
        assert_eq!(
            LintSeverity::from_str("Warning").unwrap(),
            LintSeverity::Warning
        );
        assert_eq!(
            LintSeverity::from_str("Error").unwrap(),
            LintSeverity::Error
        );

        // Aliases
        assert_eq!(LintSeverity::from_str("hint").unwrap(), LintSeverity::Info);
        assert_eq!(
            LintSeverity::from_str("warn").unwrap(),
            LintSeverity::Warning
        );
        assert_eq!(LintSeverity::from_str("err").unwrap(), LintSeverity::Error);

        // Invalid
        assert!(LintSeverity::from_str("invalid").is_err());
    }

    #[test]
    fn test_opportunity_severity_override() {
        let opp = SuggestOpportunity::new(
            OpportunityId::new(1),
            vec![],
            SuggestLocation::for_test("test.rs", "Test"),
            "Test message",
            0.8,
            OpportunityContext::Lint {
                code: "RL001".into(),
                rule: "no-unwrap".into(),
                severity: LintSeverity::Warning,
                suggestion: None,
                expected: None,
                actual: None,
            },
        );

        // Original severity
        assert_eq!(opp.lint_severity(), Some(LintSeverity::Warning));

        // Override severity
        let opp = opp.with_severity_override(LintSeverity::Error);
        assert_eq!(opp.lint_severity(), Some(LintSeverity::Error));

        // Non-lint context is unchanged
        let opp2 = SuggestOpportunity::new(
            OpportunityId::new(2),
            vec![],
            SuggestLocation::for_test("test.rs", "Test"),
            "Test message",
            0.8,
            OpportunityContext::Derive {
                derive_name: "Default".into(),
                missing_impls: vec![],
            },
        );
        let opp2 = opp2.with_severity_override(LintSeverity::Error);
        assert_eq!(opp2.lint_severity(), None); // Still None, not Lint context
    }
}