From 1099a20c5b33f6322b9fdca5ecd4c143f10c541b Mon Sep 17 00:00:00 2001 From: sehwan505 Date: Fri, 12 Dec 2025 09:41:26 +0000 Subject: [PATCH 1/2] feat: implement mode-based execution for LLM validation --- internal/llm/claudecode/provider.go | 6 + internal/llm/geminicli/provider.go | 6 + internal/llm/llm.go | 34 +++- internal/llm/openaiapi/provider.go | 6 + internal/validator/execution_unit.go | 231 ++++++++++++++++++++++++++ internal/validator/validator.go | 102 ++++++++---- internal/validator/validator_test.go | 238 +++++++++++++++++++++++++++ 7 files changed, 591 insertions(+), 32 deletions(-) diff --git a/internal/llm/claudecode/provider.go b/internal/llm/claudecode/provider.go index 4d17597..86580e7 100644 --- a/internal/llm/claudecode/provider.go +++ b/internal/llm/claudecode/provider.go @@ -38,6 +38,12 @@ func init() { {ID: "haiku", DisplayName: "haiku", Description: "Fast and efficient", Recommended: false}, }, APIKey: llm.APIKeyConfig{Required: false}, + Mode: llm.ModeAgenticSingle, + Profile: llm.ProviderProfile{ + MaxPromptChars: 100000, + DefaultTimeoutSec: 300, + MaxRetries: 1, + }, }) } diff --git a/internal/llm/geminicli/provider.go b/internal/llm/geminicli/provider.go index bb7fe3b..b51ad0b 100644 --- a/internal/llm/geminicli/provider.go +++ b/internal/llm/geminicli/provider.go @@ -37,6 +37,12 @@ func init() { {ID: "gemini-2.5-pro", DisplayName: "2.5 pro", Description: "Higher capability", Recommended: false}, }, APIKey: llm.APIKeyConfig{Required: false}, + Mode: llm.ModeAgenticSingle, + Profile: llm.ProviderProfile{ + MaxPromptChars: 100000, + DefaultTimeoutSec: 300, + MaxRetries: 1, + }, }) } diff --git a/internal/llm/llm.go b/internal/llm/llm.go index 3d8fb55..f280302 100644 --- a/internal/llm/llm.go +++ b/internal/llm/llm.go @@ -34,6 +34,32 @@ const ( XML ResponseFormat = "xml" ) +// the execution strategy for LLM-based validation. +type ProviderMode string + +const ( + // ModeParallelAPI is for traditional API providers (OpenAI, Gemini API). + ModeParallelAPI ProviderMode = "parallel_api" + + // ModeAgenticSingle is for agentic CLI tools (Claude Code, Gemini CLI). + ModeAgenticSingle ProviderMode = "agentic_single" +) + +// ProviderProfile contains mode-specific configuration for execution strategy. +type ProviderProfile struct { + // MaxPromptChars is the maximum prompt length before truncation. + MaxPromptChars int + + // DefaultTimeoutSec is the default timeout per request in seconds. + DefaultTimeoutSec int + + // MaxRetries is the maximum retry attempts on transient failures. + MaxRetries int + + // ResponseFormatHint suggests the expected response structure to the LLM. + ResponseFormatHint string +} + // String returns the string representation of the format. func (f ResponseFormat) String() string { return string(f) @@ -67,7 +93,9 @@ type ProviderInfo struct { DisplayName string DefaultModel string Available bool - Path string // CLI path or empty for API providers - Models []ModelInfo // Available models for this provider - APIKey APIKeyConfig // API key configuration + Path string // CLI path or empty for API providers + Models []ModelInfo // Available models for this provider + APIKey APIKeyConfig // API key configuration + Mode ProviderMode // Execution mode (agentic_single or parallel_api) + Profile ProviderProfile // Mode-specific execution profile } diff --git a/internal/llm/openaiapi/provider.go b/internal/llm/openaiapi/provider.go index 1c0d31f..fbbe6ca 100644 --- a/internal/llm/openaiapi/provider.go +++ b/internal/llm/openaiapi/provider.go @@ -47,6 +47,12 @@ func init() { EnvVarName: "OPENAI_API_KEY", Prefix: "sk-", }, + Mode: llm.ModeParallelAPI, + Profile: llm.ProviderProfile{ + MaxPromptChars: 8000, + DefaultTimeoutSec: 60, + MaxRetries: 2, + }, }) } diff --git a/internal/validator/execution_unit.go b/internal/validator/execution_unit.go index 8681fb9..aeb1b25 100644 --- a/internal/validator/execution_unit.go +++ b/internal/validator/execution_unit.go @@ -269,3 +269,234 @@ func (u *llmExecutionUnit) GetFiles() []string { } return nil } + +// agenticLLMExecutionUnit handles all LLM rules in a single call for agentic providers. +// Unlike llmExecutionUnit which runs (file × rule) in parallel, +// this sends all files, changes, and rules in one comprehensive prompt, +// leveraging the agent's internal capabilities (e.g., Claude Code, Gemini CLI). +type agenticLLMExecutionUnit struct { + rules []schema.PolicyRule + changes []git.Change + provider llm.Provider + policy *schema.CodePolicy + profile llm.ProviderProfile + verbose bool +} + +// Execute runs the agentic validation with all rules and changes in a single call. +func (u *agenticLLMExecutionUnit) Execute(ctx context.Context) ([]Violation, error) { + if u.provider == nil { + return nil, fmt.Errorf("LLM provider not configured") + } + + if len(u.changes) == 0 || len(u.rules) == 0 { + return nil, nil + } + + // Build comprehensive prompt with all context + prompt := u.buildAgenticPrompt() + + // Truncate if exceeds max prompt chars + if u.profile.MaxPromptChars > 0 && len(prompt) > u.profile.MaxPromptChars { + prompt = prompt[:u.profile.MaxPromptChars-100] + "\n\n... (truncated due to length limit)" + } + + if u.verbose { + fmt.Printf(" Agentic validation: %d rule(s), %d file(s), prompt %d chars\n", + len(u.rules), len(u.changes), len(prompt)) + } + + // Execute single LLM call + response, err := u.provider.Execute(ctx, prompt, llm.JSON) + if err != nil { + return nil, fmt.Errorf("agentic validation failed: %w", err) + } + + // Parse the comprehensive response + violations := u.parseAgenticResponse(response) + + return violations, nil +} + +// buildAgenticPrompt creates a comprehensive prompt for agentic validation. +func (u *agenticLLMExecutionUnit) buildAgenticPrompt() string { + var sb strings.Builder + + // System instructions + sb.WriteString(`You are a strict code reviewer. Your job is to check if code changes violate coding conventions. + +IMPORTANT INSTRUCTIONS: +1. Be CONSERVATIVE - only report violations when you are CERTAIN the code violates the rule +2. Do NOT report false positives - if unsure, report as NOT violating +3. Consider the context of the code when making your decision +4. Check ALL rules against ALL changed files +5. Return results as a JSON array + +You MUST respond with ONLY a valid JSON array (no markdown, no explanation outside JSON): +[ + { + "rule_id": "rule-id-here", + "file": "path/to/file.ext", + "violates": true, + "confidence": "high", + "description": "Brief explanation of the violation", + "suggestion": "How to fix the violation" + } +] + +If no violations are found, return an empty array: [] + +Confidence levels: +- "high": You are certain this is a violation +- "medium": Likely a violation but some uncertainty +- "low": Possible violation but significant uncertainty (will be ignored) + +`) + + // Rules section + sb.WriteString("=== RULES TO CHECK ===\n\n") + for i, rule := range u.rules { + sb.WriteString(fmt.Sprintf("Rule %d: [%s] (severity: %s)\n", i+1, rule.ID, rule.Severity)) + sb.WriteString(fmt.Sprintf(" Description: %s\n", rule.Desc)) + if rule.Category != "" { + sb.WriteString(fmt.Sprintf(" Category: %s\n", rule.Category)) + } + if rule.When != nil && len(rule.When.Languages) > 0 { + sb.WriteString(fmt.Sprintf(" Applies to: %s\n", strings.Join(rule.When.Languages, ", "))) + } + sb.WriteString("\n") + } + + // Changes section + sb.WriteString("=== FILES AND CHANGES TO REVIEW ===\n\n") + for _, change := range u.changes { + if change.Status == "D" { + continue // Skip deleted files + } + + sb.WriteString(fmt.Sprintf("--- File: %s (status: %s) ---\n", change.FilePath, change.Status)) + + // Extract and include added/modified lines + addedLines := git.ExtractAddedLines(change.Diff) + if len(addedLines) == 0 && strings.TrimSpace(change.Diff) != "" { + addedLines = strings.Split(change.Diff, "\n") + } + + if len(addedLines) > 0 { + code := strings.Join(addedLines, "\n") + // Truncate individual file content if too long + const maxFileCodeLen = 5000 + if len(code) > maxFileCodeLen { + code = code[:maxFileCodeLen] + "\n... (file content truncated)" + } + sb.WriteString(code) + sb.WriteString("\n") + } + sb.WriteString("\n") + } + + sb.WriteString("=== END OF FILES ===\n\n") + sb.WriteString("Analyze ALL files against ALL rules. Report only confirmed violations as JSON array.") + + return sb.String() +} + +// agenticViolationResponse represents a single violation in the agentic response +type agenticViolationResponse struct { + RuleID string `json:"rule_id"` + File string `json:"file"` + Violates bool `json:"violates"` + Confidence string `json:"confidence"` + Description string `json:"description"` + Suggestion string `json:"suggestion"` +} + +// parseAgenticResponse parses the JSON array response from agentic validation +func (u *agenticLLMExecutionUnit) parseAgenticResponse(response string) []Violation { + var violations []Violation + + // Clean up response - remove markdown fences if present + response = strings.TrimSpace(response) + response = strings.TrimPrefix(response, "```json") + response = strings.TrimPrefix(response, "```") + response = strings.TrimSuffix(response, "```") + response = strings.TrimSpace(response) + + // Find JSON array in response + startIdx := strings.Index(response, "[") + endIdx := strings.LastIndex(response, "]") + if startIdx == -1 || endIdx == -1 || endIdx <= startIdx { + return violations + } + + jsonStr := response[startIdx : endIdx+1] + + // Parse JSON array + var results []agenticViolationResponse + if err := json.Unmarshal([]byte(jsonStr), &results); err != nil { + // Try parsing as single object (fallback) + var single agenticViolationResponse + if err := json.Unmarshal([]byte(jsonStr), &single); err == nil { + results = []agenticViolationResponse{single} + } else { + return violations + } + } + + // Convert to Violation structs + for _, r := range results { + // Skip non-violations and low-confidence results + if !r.Violates || r.Confidence == "low" { + continue + } + + // Find the matching rule for severity + severity := "warning" + for _, rule := range u.rules { + if rule.ID == r.RuleID { + severity = rule.Severity + break + } + } + + message := r.Description + if r.Suggestion != "" { + message += fmt.Sprintf(" | Suggestion: %s", r.Suggestion) + } + + violations = append(violations, Violation{ + RuleID: r.RuleID, + Severity: severity, + Message: message, + File: r.File, + ToolName: "llm-validator", + }) + } + + return violations +} + +// GetRuleIDs returns all rule IDs in this execution unit +func (u *agenticLLMExecutionUnit) GetRuleIDs() []string { + ids := make([]string, len(u.rules)) + for i, rule := range u.rules { + ids[i] = rule.ID + } + return ids +} + +// GetEngineName returns "llm-validator" +func (u *agenticLLMExecutionUnit) GetEngineName() string { + return "llm-validator" +} + +// GetFiles returns all files in this execution unit +func (u *agenticLLMExecutionUnit) GetFiles() []string { + files := make([]string, 0, len(u.changes)) + for _, change := range u.changes { + if change.Status != "D" { + files = append(files, change.FilePath) + } + } + return files +} diff --git a/internal/validator/validator.go b/internal/validator/validator.go index 3bc2c76..758153c 100644 --- a/internal/validator/validator.go +++ b/internal/validator/validator.go @@ -35,14 +35,15 @@ type Violation struct { // Validator validates code against policy using adapters directly // This replaces the old engine-based architecture type Validator struct { - policy *schema.CodePolicy - verbose bool - linterRegistry *linter.Registry - workDir string - symDir string // .sym directory for config files - ctx context.Context - ctxCancel context.CancelFunc - llmProvider llm.Provider + policy *schema.CodePolicy + verbose bool + linterRegistry *linter.Registry + workDir string + symDir string // .sym directory for config files + ctx context.Context + ctxCancel context.CancelFunc + llmProvider llm.Provider + llmProviderInfo *llm.ProviderInfo // Provider metadata including mode } // NewValidator creates a new adapter-based validator @@ -88,6 +89,10 @@ func NewValidatorWithWorkDir(policy *schema.CodePolicy, verbose bool, workDir st // SetLLMProvider sets the LLM provider for this validator func (v *Validator) SetLLMProvider(provider llm.Provider) { v.llmProvider = provider + // Also store provider info for mode-based execution decisions + if provider != nil { + v.llmProviderInfo = llm.GetProviderInfo(provider.Name()) + } } // getEngineName extracts the engine name from a rule @@ -175,31 +180,17 @@ func (v *Validator) groupRulesByEngine(rules []schema.PolicyRule, changes []git. } // createExecutionUnits creates execution units from rule groups -// - Linter: {모든 파일, 모든 규칙} = 1개 단위 -// - LLM: {파일 1개, 규칙 1개} = 파일수 × 규칙수 단위 -func (v *Validator) createExecutionUnits(groups map[string]*ruleGroup, changes []git.Change) []executionUnit { +// - Linter: all files + all rules = 1 unit +// - LLM (agentic_single mode): all files + all rules = 1 unit (Claude Code, Gemini CLI) +// - LLM (parallel_api mode): 1 file × 1 rule = N units (OpenAI API) +func (v *Validator) createExecutionUnits(groups map[string]*ruleGroup) []executionUnit { var units []executionUnit for engineName, group := range groups { if engineName == "llm-validator" { - // LLM: 각 (파일, 규칙) 쌍이 별도 실행 단위 - for _, rule := range group.rules { - ruleChanges := v.filterChangesForRule(group.changes, &rule) - for _, change := range ruleChanges { - if change.Status == "D" { - continue - } - units = append(units, &llmExecutionUnit{ - rule: rule, - change: change, - provider: v.llmProvider, - policy: v.policy, - verbose: v.verbose, - }) - } - } + units = append(units, v.createLLMExecutionUnits(group)...) } else { - // Linter: 모든 파일, 모든 규칙 = 1개 실행 단위 + // Linter: all files + all rules = 1 unit files := make([]string, 0, len(group.files)) for f := range group.files { files = append(files, f) @@ -218,6 +209,59 @@ func (v *Validator) createExecutionUnits(groups map[string]*ruleGroup, changes [ return units } +// createLLMExecutionUnits creates execution units for LLM validation +// based on the provider's mode (agentic_single vs parallel_api) +func (v *Validator) createLLMExecutionUnits(group *ruleGroup) []executionUnit { + var units []executionUnit + + // Check provider mode (default to parallel for backward compatibility) + mode := llm.ModeParallelAPI + var profile llm.ProviderProfile + + if v.llmProviderInfo != nil { + mode = v.llmProviderInfo.Mode + profile = v.llmProviderInfo.Profile + } + + switch mode { + case llm.ModeAgenticSingle: + // Agentic mode: all files + all rules = 1 unit + // Agent-based CLIs (Claude Code, Gemini CLI) can handle complex tasks + // internally, so we send everything in a single call + if len(group.changes) > 0 && len(group.rules) > 0 { + units = append(units, &agenticLLMExecutionUnit{ + rules: group.rules, + changes: group.changes, + provider: v.llmProvider, + policy: v.policy, + profile: profile, + verbose: v.verbose, + }) + } + + default: // ModeParallelAPI + // Parallel API mode: 1 file × 1 rule = separate units + // Traditional APIs (OpenAI) process multiple parallel requests + for _, rule := range group.rules { + ruleChanges := v.filterChangesForRule(group.changes, &rule) + for _, change := range ruleChanges { + if change.Status == "D" { + continue + } + units = append(units, &llmExecutionUnit{ + rule: rule, + change: change, + provider: v.llmProvider, + policy: v.policy, + verbose: v.verbose, + }) + } + } + } + + return units +} + // executeUnitsParallel executes units in parallel with semaphore-based concurrency func (v *Validator) executeUnitsParallel(ctx context.Context, units []executionUnit) ([]Violation, []ValidationError) { var wg sync.WaitGroup @@ -327,7 +371,7 @@ func (v *Validator) ValidateChanges(ctx context.Context, changes []git.Change) ( groups := v.groupRulesByEngine(v.policy.Rules, changes) // Phase 3: Create execution units - units := v.createExecutionUnits(groups, changes) + units := v.createExecutionUnits(groups) if v.verbose { // Count files to check diff --git a/internal/validator/validator_test.go b/internal/validator/validator_test.go index 64f095f..c428f29 100644 --- a/internal/validator/validator_test.go +++ b/internal/validator/validator_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/DevSymphony/sym-cli/internal/linter" + "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/internal/util/git" "github.com/DevSymphony/sym-cli/pkg/schema" "github.com/stretchr/testify/assert" @@ -474,3 +475,240 @@ func TestGroupRulesByEngine(t *testing.T) { assert.True(t, groups["eslint"].files["main.go"]) }) } + +func TestCreateLLMExecutionUnits_ModeBasedBranching(t *testing.T) { + changes := []git.Change{ + {FilePath: "app.js", Status: "M", Diff: "+console.log('test')"}, + {FilePath: "main.py", Status: "A", Diff: "+print('hello')"}, + } + rules := []schema.PolicyRule{ + {ID: "rule-1", Enabled: true, Severity: "error", Desc: "No console.log"}, + {ID: "rule-2", Enabled: true, Severity: "warning", Desc: "No print statements"}, + } + group := &ruleGroup{ + engineName: "llm-validator", + rules: rules, + changes: changes, + files: map[string]bool{"app.js": true, "main.py": true}, + } + + t.Run("parallel_api mode creates multiple units (file x rule)", func(t *testing.T) { + v := &Validator{ + llmProviderInfo: &llm.ProviderInfo{ + Mode: llm.ModeParallelAPI, + Profile: llm.ProviderProfile{ + MaxPromptChars: 8000, + }, + }, + } + units := v.createLLMExecutionUnits(group) + + // 2 files x 2 rules = 4 units (parallel API mode) + assert.Len(t, units, 4) + for _, unit := range units { + assert.Equal(t, "llm-validator", unit.GetEngineName()) + assert.Len(t, unit.GetRuleIDs(), 1) // Each unit has 1 rule + assert.Len(t, unit.GetFiles(), 1) // Each unit has 1 file + } + }) + + t.Run("agentic_single mode creates single unit (all files, all rules)", func(t *testing.T) { + v := &Validator{ + llmProviderInfo: &llm.ProviderInfo{ + Mode: llm.ModeAgenticSingle, + Profile: llm.ProviderProfile{ + MaxPromptChars: 100000, + DefaultTimeoutSec: 300, + }, + }, + } + units := v.createLLMExecutionUnits(group) + + // Agentic mode: 1 unit with all files and rules + assert.Len(t, units, 1) + assert.Equal(t, "llm-validator", units[0].GetEngineName()) + assert.Len(t, units[0].GetRuleIDs(), 2) // All rules in single unit + assert.Len(t, units[0].GetFiles(), 2) // All files in single unit + }) + + t.Run("nil provider info defaults to parallel_api mode", func(t *testing.T) { + v := &Validator{ + llmProviderInfo: nil, // No provider info + } + units := v.createLLMExecutionUnits(group) + + // Default (parallel): 2 files x 2 rules = 4 units + assert.Len(t, units, 4) + }) + + t.Run("empty changes returns no units", func(t *testing.T) { + emptyGroup := &ruleGroup{ + engineName: "llm-validator", + rules: rules, + changes: []git.Change{}, + files: map[string]bool{}, + } + v := &Validator{ + llmProviderInfo: &llm.ProviderInfo{Mode: llm.ModeAgenticSingle}, + } + units := v.createLLMExecutionUnits(emptyGroup) + assert.Len(t, units, 0) + }) + + t.Run("empty rules returns no units", func(t *testing.T) { + noRulesGroup := &ruleGroup{ + engineName: "llm-validator", + rules: []schema.PolicyRule{}, + changes: changes, + files: map[string]bool{"app.js": true}, + } + v := &Validator{ + llmProviderInfo: &llm.ProviderInfo{Mode: llm.ModeAgenticSingle}, + } + units := v.createLLMExecutionUnits(noRulesGroup) + assert.Len(t, units, 0) + }) +} + +func TestAgenticLLMExecutionUnit_Getters(t *testing.T) { + rules := []schema.PolicyRule{ + {ID: "rule-1", Severity: "error"}, + {ID: "rule-2", Severity: "warning"}, + } + changes := []git.Change{ + {FilePath: "app.js", Status: "M"}, + {FilePath: "main.py", Status: "A"}, + {FilePath: "deleted.go", Status: "D"}, // Should be excluded from GetFiles + } + + unit := &agenticLLMExecutionUnit{ + rules: rules, + changes: changes, + } + + t.Run("GetRuleIDs returns all rule IDs", func(t *testing.T) { + ids := unit.GetRuleIDs() + assert.Equal(t, []string{"rule-1", "rule-2"}, ids) + }) + + t.Run("GetEngineName returns llm-validator", func(t *testing.T) { + assert.Equal(t, "llm-validator", unit.GetEngineName()) + }) + + t.Run("GetFiles excludes deleted files", func(t *testing.T) { + files := unit.GetFiles() + assert.Len(t, files, 2) + assert.Contains(t, files, "app.js") + assert.Contains(t, files, "main.py") + assert.NotContains(t, files, "deleted.go") + }) +} + +func TestAgenticLLMExecutionUnit_BuildPrompt(t *testing.T) { + rules := []schema.PolicyRule{ + {ID: "security-1", Severity: "error", Desc: "No hardcoded secrets", Category: "security"}, + {ID: "style-1", Severity: "warning", Desc: "Use const instead of let", When: &schema.Selector{Languages: []string{"javascript"}}}, + } + changes := []git.Change{ + {FilePath: "app.js", Status: "M", Diff: "+const API_KEY = 'secret123';"}, + {FilePath: "main.py", Status: "A", Diff: "+password = 'admin'"}, + } + + unit := &agenticLLMExecutionUnit{ + rules: rules, + changes: changes, + profile: llm.ProviderProfile{MaxPromptChars: 100000}, + } + + prompt := unit.buildAgenticPrompt() + + // Check prompt contains key sections + assert.Contains(t, prompt, "=== RULES TO CHECK ===") + assert.Contains(t, prompt, "=== FILES AND CHANGES TO REVIEW ===") + assert.Contains(t, prompt, "=== END OF FILES ===") + + // Check rules are included + assert.Contains(t, prompt, "security-1") + assert.Contains(t, prompt, "No hardcoded secrets") + assert.Contains(t, prompt, "style-1") + assert.Contains(t, prompt, "Use const instead of let") + + // Check files are included + assert.Contains(t, prompt, "app.js") + assert.Contains(t, prompt, "main.py") + + // Check JSON format instructions + assert.Contains(t, prompt, "JSON array") + assert.Contains(t, prompt, "rule_id") +} + +func TestAgenticLLMExecutionUnit_ParseResponse(t *testing.T) { + rules := []schema.PolicyRule{ + {ID: "security-1", Severity: "error"}, + {ID: "style-1", Severity: "warning"}, + } + unit := &agenticLLMExecutionUnit{rules: rules} + + t.Run("parses valid JSON array response", func(t *testing.T) { + response := `[ + {"rule_id": "security-1", "file": "app.js", "violates": true, "confidence": "high", "description": "Found hardcoded secret", "suggestion": "Use environment variables"}, + {"rule_id": "style-1", "file": "main.py", "violates": true, "confidence": "medium", "description": "Should use const", "suggestion": "Change let to const"} + ]` + violations := unit.parseAgenticResponse(response) + + assert.Len(t, violations, 2) + assert.Equal(t, "security-1", violations[0].RuleID) + assert.Equal(t, "error", violations[0].Severity) + assert.Equal(t, "app.js", violations[0].File) + assert.Contains(t, violations[0].Message, "Found hardcoded secret") + assert.Contains(t, violations[0].Message, "Use environment variables") + }) + + t.Run("filters out low confidence results", func(t *testing.T) { + response := `[ + {"rule_id": "security-1", "file": "app.js", "violates": true, "confidence": "high", "description": "High confidence"}, + {"rule_id": "style-1", "file": "main.py", "violates": true, "confidence": "low", "description": "Low confidence - ignored"} + ]` + violations := unit.parseAgenticResponse(response) + + assert.Len(t, violations, 1) + assert.Equal(t, "security-1", violations[0].RuleID) + }) + + t.Run("filters out non-violations", func(t *testing.T) { + response := `[ + {"rule_id": "security-1", "file": "app.js", "violates": true, "confidence": "high", "description": "Violation"}, + {"rule_id": "style-1", "file": "main.py", "violates": false, "confidence": "high", "description": "Not a violation"} + ]` + violations := unit.parseAgenticResponse(response) + + assert.Len(t, violations, 1) + assert.Equal(t, "security-1", violations[0].RuleID) + }) + + t.Run("handles empty array", func(t *testing.T) { + response := `[]` + violations := unit.parseAgenticResponse(response) + assert.Len(t, violations, 0) + }) + + t.Run("handles markdown-wrapped JSON", func(t *testing.T) { + response := "```json\n[{\"rule_id\": \"security-1\", \"file\": \"app.js\", \"violates\": true, \"confidence\": \"high\", \"description\": \"test\"}]\n```" + violations := unit.parseAgenticResponse(response) + assert.Len(t, violations, 1) + }) + + t.Run("handles invalid JSON gracefully", func(t *testing.T) { + response := `not valid json` + violations := unit.parseAgenticResponse(response) + assert.Len(t, violations, 0) + }) + + t.Run("uses default severity for unknown rule", func(t *testing.T) { + response := `[{"rule_id": "unknown-rule", "file": "app.js", "violates": true, "confidence": "high", "description": "test"}]` + violations := unit.parseAgenticResponse(response) + + assert.Len(t, violations, 1) + assert.Equal(t, "warning", violations[0].Severity) // Default severity + }) +} From 29537769a66337bad940926b30b982cc989bd549 Mon Sep 17 00:00:00 2001 From: sehwan505 Date: Fri, 12 Dec 2025 09:50:32 +0000 Subject: [PATCH 2/2] fix: standardize formatting for LLM provider mode initialization --- internal/llm/claudecode/provider.go | 2 +- internal/llm/geminicli/provider.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/llm/claudecode/provider.go b/internal/llm/claudecode/provider.go index 86580e7..83d5e16 100644 --- a/internal/llm/claudecode/provider.go +++ b/internal/llm/claudecode/provider.go @@ -38,7 +38,7 @@ func init() { {ID: "haiku", DisplayName: "haiku", Description: "Fast and efficient", Recommended: false}, }, APIKey: llm.APIKeyConfig{Required: false}, - Mode: llm.ModeAgenticSingle, + Mode: llm.ModeAgenticSingle, Profile: llm.ProviderProfile{ MaxPromptChars: 100000, DefaultTimeoutSec: 300, diff --git a/internal/llm/geminicli/provider.go b/internal/llm/geminicli/provider.go index b51ad0b..90c7c3f 100644 --- a/internal/llm/geminicli/provider.go +++ b/internal/llm/geminicli/provider.go @@ -37,7 +37,7 @@ func init() { {ID: "gemini-2.5-pro", DisplayName: "2.5 pro", Description: "Higher capability", Recommended: false}, }, APIKey: llm.APIKeyConfig{Required: false}, - Mode: llm.ModeAgenticSingle, + Mode: llm.ModeAgenticSingle, Profile: llm.ProviderProfile{ MaxPromptChars: 100000, DefaultTimeoutSec: 300,