diff --git a/cmd/test-linter/main.go b/cmd/test-linter/main.go new file mode 100644 index 0000000..a344d89 --- /dev/null +++ b/cmd/test-linter/main.go @@ -0,0 +1,123 @@ +package main + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/DevSymphony/sym-cli/internal/adapter" + "github.com/DevSymphony/sym-cli/internal/adapter/eslint" +) + +func main() { + fmt.Println("=== Testing ESLint Adapter ===") + fmt.Println() + + // 1. Create ESLint adapter + homeDir, _ := os.UserHomeDir() + toolsDir := filepath.Join(homeDir, ".sym", "tools") + workDir, _ := os.Getwd() + + adp := eslint.NewAdapter(toolsDir) + fmt.Printf("✓ Created ESLint adapter\n") + fmt.Printf(" Tools directory: %s\n", adp.ToolsDir) + + // 2. Check availability + ctx := context.Background() + fmt.Println("Checking ESLint availability...") + err := adp.CheckAvailability(ctx) + if err != nil { + fmt.Printf("⚠️ ESLint not available: %v\n", err) + fmt.Println("\nInstalling ESLint...") + // Try to install + installConfig := adapter.InstallConfig{ + ToolsDir: toolsDir, + } + installErr := adp.Install(ctx, installConfig) + if installErr != nil { + fmt.Printf("❌ Failed to install: %v\n", installErr) + os.Exit(1) + } + fmt.Println("✓ ESLint installed successfully") + fmt.Println() + } else { + fmt.Println("✓ ESLint is available") + fmt.Println() + } + + // 3. Create a simple ESLint config + config := []byte(`{ + "env": { + "node": true, + "es2021": true + }, + "extends": "eslint:recommended", + "parserOptions": { + "ecmaVersion": 12 + }, + "rules": { + "semi": ["error", "always"], + "quotes": ["error", "single"], + "no-unused-vars": "error", + "no-console": "warn" + } + }`) + + // 4. Find test file + testFile := filepath.Join(workDir, "test_file.js") + if _, err := os.Stat(testFile); os.IsNotExist(err) { + fmt.Printf("❌ Test file not found: %s\n", testFile) + os.Exit(1) + } + fmt.Printf("Test file: %s\n\n", testFile) + + // 5. Execute ESLint + fmt.Println("Running ESLint...") + output, err := adp.Execute(ctx, config, []string{testFile}) + if err != nil { + fmt.Printf("⚠️ ESLint execution error: %v\n", err) + // Continue to parse output even if there's an error (violations cause non-zero exit) + } + + // 6. Show raw output + fmt.Println("\n--- Raw ESLint Output ---") + if output.Stdout != "" { + fmt.Printf("STDOUT:\n%s\n", output.Stdout) + } + if output.Stderr != "" { + fmt.Printf("STDERR:\n%s\n", output.Stderr) + } + fmt.Printf("Exit Code: %d\n", output.ExitCode) + fmt.Printf("Duration: %s\n", output.Duration) + + // 7. Parse violations + fmt.Println("\n--- Parsed Violations ---") + violations, parseErr := adp.ParseOutput(output) + if parseErr != nil { + fmt.Printf("❌ Failed to parse output: %v\n", parseErr) + os.Exit(1) + } + + if len(violations) == 0 { + fmt.Println("✅ No violations found!") + } else { + fmt.Printf("Found %d violation(s):\n\n", len(violations)) + for i, v := range violations { + fmt.Printf("%d. [%s] %s\n", i+1, v.Severity, v.RuleID) + fmt.Printf(" File: %s:%d:%d\n", v.File, v.Line, v.Column) + fmt.Printf(" Message: %s\n", v.Message) + + // Show that we have the raw output stored + if len(output.Stdout) > 0 { + fmt.Printf(" ✓ Raw output captured: %d bytes\n", len(output.Stdout)) + } + if len(output.Stderr) > 0 { + fmt.Printf(" ✓ Raw error captured: %d bytes\n", len(output.Stderr)) + } + fmt.Printf(" ✓ Execution time: %s\n\n", output.Duration) + } + } + + fmt.Println("=== Test Complete ===") +} diff --git a/go.mod b/go.mod index eaae966..a28b7b9 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/DevSymphony/sym-cli go 1.25.1 require ( - github.com/bmatcuk/doublestar/v4 v4.9.1 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 // symphonyclient integration: browser automation for OAuth github.com/spf13/cobra v1.10.1 ) diff --git a/go.sum b/go.sum index d534903..c34c37e 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -github.com/bmatcuk/doublestar/v4 v4.9.1 h1:X8jg9rRZmJd4yRy7ZeNDRnM+T3ZfHv15JiBJ/avrEXE= -github.com/bmatcuk/doublestar/v4 v4.9.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc= github.com/chzyer/logex v1.1.10 h1:Swpa1K6QvQznwJRcfTfQJmTE72DqScAa40E+fbHEXEE= github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e h1:fY5BOSpyZCqRo5OhCuC+XN+r/bBCmeuuJtjz+bCNIf8= @@ -15,10 +13,10 @@ github.com/google/jsonschema-go v0.3.0 h1:6AH2TxVNtk3IlvkkhjrtbUc4S8AvO0Xii0DxIy github.com/google/jsonschema-go v0.3.0/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/modelcontextprotocol/go-sdk v1.1.0 h1:Qjayg53dnKC4UZ+792W21e4BpwEZBzwgRW6LrjLWSwA= -github.com/modelcontextprotocol/go-sdk v1.1.0/go.mod h1:6fM3LCm3yV7pAs8isnKLn07oKtB0MP9LHd3DfAcKw10= github.com/manifoldco/promptui v0.9.0 h1:3V4HzJk1TtXW1MTZMP7mdlwbBpIinw3HztaIlYthEiA= github.com/manifoldco/promptui v0.9.0/go.mod h1:ka04sppxSGFAtxX0qhlYQjISsg9mR4GWtQEhdbn6Pgg= +github.com/modelcontextprotocol/go-sdk v1.1.0 h1:Qjayg53dnKC4UZ+792W21e4BpwEZBzwgRW6LrjLWSwA= +github.com/modelcontextprotocol/go-sdk v1.1.0/go.mod h1:6fM3LCm3yV7pAs8isnKLn07oKtB0MP9LHd3DfAcKw10= github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU= github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= @@ -35,13 +33,12 @@ github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zI github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4= golang.org/x/oauth2 v0.30.0 h1:dnDm7JmhM45NNpd8FDDeLhK6FwqbOf4MLCM9zb1BOHI= golang.org/x/oauth2 v0.30.0/go.mod h1:B++QgG3ZKulg6sRPGD/mqlHQs5rB3Ml9erfeDY7xKlU= -golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/tools v0.34.0 h1:qIpSLOxeCYGg9TrcJokLBG4KFA6d795g0xkBkiESGlo= -golang.org/x/tools v0.34.0/go.mod h1:pAP9OwEaY1CAW3HOmg3hLZC5Z0CCmzjAF2UQMSqNARg= golang.org/x/sys v0.0.0-20181122145206-62eef0e2fa9b/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.38.0 h1:3yZWxaJjBmCWXqhN1qh02AkOnCQ1poK6oF+a7xWL6Gc= golang.org/x/sys v0.38.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/tools v0.34.0 h1:qIpSLOxeCYGg9TrcJokLBG4KFA6d795g0xkBkiESGlo= +golang.org/x/tools v0.34.0/go.mod h1:pAP9OwEaY1CAW3HOmg3hLZC5Z0CCmzjAF2UQMSqNARg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/adapter/checkstyle/converter.go b/internal/adapter/checkstyle/converter.go index 94412c3..d4ac1c8 100644 --- a/internal/adapter/checkstyle/converter.go +++ b/internal/adapter/checkstyle/converter.go @@ -255,7 +255,8 @@ Output: userPrompt := fmt.Sprintf("Convert this Java rule to Checkstyle module:\n\n%s", rule.Say) - response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) + // Call LLM with power model + low reasoning + response, err := llmClient.Request(systemPrompt, userPrompt).WithPower(llm.ReasoningMinimal).Execute(ctx) if err != nil { return nil, fmt.Errorf("LLM call failed: %w", err) } @@ -285,6 +286,9 @@ Output: return nil, nil } + // Filter properties to only include valid ones for this module + filteredProps := filterValidProperties(result.ModuleName, result.Properties) + // Build module module := &checkstyleModule{ Name: result.ModuleName, @@ -297,8 +301,11 @@ Output: Value: mapCheckstyleSeverity(result.Severity), }) - // Add other properties - for key, value := range result.Properties { + // Add filtered properties + for key, value := range filteredProps { + if key == "severity" { + continue // Already added above + } module.Properties = append(module.Properties, checkstyleProperty{ Name: key, Value: value, @@ -321,3 +328,195 @@ func mapCheckstyleSeverity(severity string) string { return "error" } } + +// validCheckstyleProperties defines valid properties for each Checkstyle module +// This prevents LLM from generating invalid properties that cause runtime errors +var validCheckstyleProperties = map[string]map[string]bool{ + "TypeName": { + "severity": true, + "format": true, + "tokens": true, + }, + "MethodName": { + "severity": true, + "format": true, + "allowClassName": true, + "applyToPublic": true, + "applyToProtected": true, + "applyToPackage": true, + "applyToPrivate": true, + "tokens": true, + }, + "ParameterName": { + "severity": true, + "format": true, + "ignoreOverridden": true, + "accessModifiers": true, + }, + "LocalVariableName": { + "severity": true, + "format": true, + "allowOneCharVarInForLoop": true, + }, + "ConstantName": { + "severity": true, + "format": true, + "applyToPublic": true, + "applyToProtected": true, + "applyToPackage": true, + "applyToPrivate": true, + }, + "LineLength": { + "severity": true, + "max": true, + "ignorePattern": true, + "fileExtensions": true, + }, + "MethodLength": { + "severity": true, + "max": true, + "countEmpty": true, + "tokens": true, + }, + "ParameterNumber": { + "severity": true, + "max": true, + "ignoreOverriddenMethods": true, + "tokens": true, + }, + "FileLength": { + "severity": true, + "max": true, + "fileExtensions": true, + }, + "Indentation": { + "severity": true, + "basicOffset": true, + "braceAdjustment": true, + "caseIndent": true, + "throwsIndent": true, + "arrayInitIndent": true, + "lineWrappingIndentation": true, + "forceStrictCondition": true, + }, + "WhitespaceAround": { + "severity": true, + "allowEmptyConstructors": true, + "allowEmptyMethods": true, + "allowEmptyTypes": true, + "allowEmptyLoops": true, + "allowEmptyLambdas": true, + "allowEmptyCatches": true, + "ignoreEnhancedForColon": true, + "tokens": true, + }, + "NeedBraces": { + "severity": true, + "allowSingleLineStatement": true, + "allowEmptyLoopBody": true, + "tokens": true, + }, + "LeftCurly": { + "severity": true, + "option": true, + "ignoreEnums": true, + "tokens": true, + }, + "RightCurly": { + "severity": true, + "option": true, + "tokens": true, + }, + "AvoidStarImport": { + "severity": true, + "excludes": true, + "allowClassImports": true, + "allowStaticMemberImports": true, + }, + "IllegalImport": { + "severity": true, + "illegalPkgs": true, + "illegalClasses": true, + "regexp": true, + }, + "UnusedImports": { + "severity": true, + "processJavadoc": true, + }, + "CyclomaticComplexity": { + "severity": true, + "max": true, + "switchBlockAsSingleDecisionPoint": true, + "tokens": true, + }, + "NPathComplexity": { + "severity": true, + "max": true, + }, + "JavadocMethod": { + "severity": true, + "accessModifiers": true, + "allowMissingParamTags": true, + "allowMissingReturnTag": true, + "allowedAnnotations": true, + "validateThrows": true, + "tokens": true, + }, + "JavadocType": { + "severity": true, + "scope": true, + "excludeScope": true, + "authorFormat": true, + "versionFormat": true, + "allowMissingParamTags": true, + "allowUnknownTags": true, + "allowedAnnotations": true, + "tokens": true, + }, + "MissingJavadocMethod": { + "severity": true, + "minLineCount": true, + "allowedAnnotations": true, + "scope": true, + "excludeScope": true, + "allowMissingPropertyJavadoc": true, + "ignoreMethodNamesRegex": true, + "tokens": true, + }, + "EmptyBlock": { + "severity": true, + "option": true, + "tokens": true, + }, + "MagicNumber": { + "severity": true, + "ignoreNumbers": true, + "ignoreHashCodeMethod": true, + "ignoreAnnotation": true, + "ignoreFieldDeclaration": true, + "ignoreAnnotationElementDefaults": true, + "constantWaiverParentToken": true, + "tokens": true, + }, +} + +// filterValidProperties filters out invalid properties for a given module +func filterValidProperties(moduleName string, properties map[string]string) map[string]string { + validProps, hasDefinedProps := validCheckstyleProperties[moduleName] + if !hasDefinedProps { + // If module is not in our whitelist, only allow severity + result := make(map[string]string) + if sev, ok := properties["severity"]; ok { + result["severity"] = sev + } + return result + } + + result := make(map[string]string) + for key, value := range properties { + if validProps[key] { + result[key] = value + } + } + return result +} diff --git a/internal/adapter/eslint/converter.go b/internal/adapter/eslint/converter.go index 07a4fa1..42d5b86 100644 --- a/internal/adapter/eslint/converter.go +++ b/internal/adapter/eslint/converter.go @@ -151,22 +151,22 @@ Return ONLY a JSON object (no markdown fences) with this structure: "options": {...} } -Common ESLint rules: -- Naming: camelcase, id-match, id-length, new-cap -- Console: no-console, no-debugger, no-alert -- Code Quality: no-unused-vars, no-undef, eqeqeq, prefer-const, no-var -- Complexity: complexity, max-depth, max-nested-callbacks, max-lines-per-function -- Length: max-len, max-lines, max-params, max-statements +Available native ESLint rules: +- Console/Debug: no-console, no-debugger, no-alert +- Variables: no-unused-vars, no-undef, no-var, prefer-const +- Naming: camelcase, new-cap, id-length, id-match +- Code Quality: eqeqeq, no-eval, no-implied-eval, no-new-func +- Complexity: complexity, max-depth, max-nested-callbacks +- Length/Size: max-len, max-lines, max-lines-per-function, max-params, max-statements - Style: indent, quotes, semi, comma-dangle, brace-style -- Imports: no-restricted-imports -- Security: no-eval, no-implied-eval, no-new-func +- Imports: no-restricted-imports, no-duplicate-imports +- Best Practices: curly, dot-notation, no-else-return, no-empty, no-empty-function, no-magic-numbers, no-throw-literal, no-useless-return, require-await -If the rule cannot be expressed in ESLint, return: -{ - "rule_name": "", - "severity": "off", - "options": null -} +CRITICAL RULES: +1. ONLY use native ESLint rules - do NOT invent or guess rule names +2. If no rule can enforce this requirement, return rule_name as empty string "" +3. Do NOT suggest plugin rules (e.g., @typescript-eslint/*, eslint-plugin-*) +4. When in doubt, return empty rule_name - it's better to skip than use wrong rule Examples: @@ -192,15 +192,33 @@ Output: "rule_name": "camelcase", "severity": "error", "options": {"properties": "always"} -}` +} + +Input: "File names must be kebab-case" +Output: +{ + "rule_name": "", + "severity": "off", + "options": null +} +(Reason: No native ESLint rule for file naming) + +Input: "No hardcoded API keys" +Output: +{ + "rule_name": "", + "severity": "off", + "options": null +} +(Reason: Requires plugin or semantic analysis)` userPrompt := fmt.Sprintf("Convert this rule to ESLint configuration:\n\n%s", rule.Say) if rule.Severity != "" { userPrompt += fmt.Sprintf("\nSeverity: %s", rule.Severity) } - // Call LLM - response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) + // Call LLM with power model + low reasoning + response, err := llmClient.Request(systemPrompt, userPrompt).WithPower(llm.ReasoningMinimal).Execute(ctx) if err != nil { return "", nil, fmt.Errorf("LLM call failed: %w", err) } diff --git a/internal/adapter/pmd/converter.go b/internal/adapter/pmd/converter.go index 9bc803f..09116c8 100644 --- a/internal/adapter/pmd/converter.go +++ b/internal/adapter/pmd/converter.go @@ -147,62 +147,67 @@ func (c *Converter) ConvertRules(ctx context.Context, rules []schema.UserRule, l func (c *Converter) convertSingleRule(ctx context.Context, rule schema.UserRule, llmClient *llm.Client) (*pmdRule, error) { systemPrompt := `You are a PMD 7.x configuration expert. Convert natural language Java coding rules to PMD rule references. -Return ONLY a JSON object (no markdown fences): +Return ONLY a JSON object with exactly these two fields (no other fields): { - "rule_ref": "category/java/CATEGORY.xml/RuleName", - "priority": 1-5 + "rule_ref": "category/java/category.xml/RuleName", + "priority": 1 } -IMPORTANT: PMD 7.x uses "category/java/" prefix, NOT "rulesets/java/". - -Common PMD 7.x rules: -- Best Practices: category/java/bestpractices.xml/UnusedPrivateMethod -- Code Style: category/java/codestyle.xml/ShortVariable -- Design: category/java/design.xml/TooManyMethods, category/java/design.xml/CyclomaticComplexity -- Error Prone: category/java/errorprone.xml/EmptyCatchBlock -- Security: category/java/security.xml/HardCodedCryptoKey - -Priority levels: 1=High, 2=Medium-High, 3=Medium, 4=Low, 5=Info - -If cannot convert, return: +Valid PMD 7.x categories and rules: +- category/java/bestpractices.xml/UnusedPrivateMethod +- category/java/bestpractices.xml/UnusedLocalVariable +- category/java/bestpractices.xml/UnusedFormalParameter +- category/java/bestpractices.xml/AvoidReassigningParameters +- category/java/codestyle.xml/ShortVariable +- category/java/codestyle.xml/LongVariable +- category/java/codestyle.xml/ShortMethodName +- category/java/codestyle.xml/ClassNamingConventions +- category/java/codestyle.xml/MethodNamingConventions +- category/java/codestyle.xml/FieldNamingConventions +- category/java/codestyle.xml/UnnecessaryImport +- category/java/design.xml/TooManyMethods +- category/java/design.xml/ExcessiveMethodLength +- category/java/design.xml/ExcessiveParameterList +- category/java/design.xml/CyclomaticComplexity +- category/java/design.xml/NPathComplexity +- category/java/design.xml/GodClass +- category/java/errorprone.xml/EmptyCatchBlock +- category/java/errorprone.xml/AvoidCatchingNPE +- category/java/errorprone.xml/EmptyIfStmt +- category/java/security.xml/HardCodedCryptoKey + +Priority: 1=High, 2=Medium-High, 3=Medium, 4=Low, 5=Info + +If the rule cannot be mapped to a valid PMD rule, return: { "rule_ref": "", "priority": 3 } -Example: - -Input: "Avoid unused private methods" -Output: -{ - "rule_ref": "category/java/bestpractices.xml/UnusedPrivateMethod", - "priority": 2 -} - -Input: "No empty catch blocks" -Output: -{ - "rule_ref": "category/java/errorprone.xml/EmptyCatchBlock", - "priority": 2 -}` +IMPORTANT: Return ONLY the JSON object. Do NOT include description, message, or any other fields.` userPrompt := fmt.Sprintf("Convert this Java rule to PMD rule reference:\n\n%s", rule.Say) - response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) + // Call LLM with power model + low reasoning + response, err := llmClient.Request(systemPrompt, userPrompt).WithPower(llm.ReasoningMinimal).Execute(ctx) if err != nil { return nil, fmt.Errorf("LLM call failed: %w", err) } - // Parse response + // Parse response - extract JSON object response = strings.TrimSpace(response) response = strings.TrimPrefix(response, "```json") response = strings.TrimPrefix(response, "```") response = strings.TrimSuffix(response, "```") response = strings.TrimSpace(response) - if response == "" { - return nil, fmt.Errorf("LLM returned empty response") + // Find JSON object boundaries to handle extra text + startIdx := strings.Index(response, "{") + endIdx := strings.LastIndex(response, "}") + if startIdx == -1 || endIdx == -1 || endIdx <= startIdx { + return nil, fmt.Errorf("no valid JSON object found in response") } + response = response[startIdx : endIdx+1] var result struct { RuleRef string `json:"rule_ref"` @@ -217,6 +222,24 @@ Output: return nil, nil } + // Validate rule_ref format: must start with "category/java/" + if !strings.HasPrefix(result.RuleRef, "category/java/") { + // Try to fix old format (rulesets/java/...) to new format (category/java/...) + if strings.HasPrefix(result.RuleRef, "rulesets/java/") { + result.RuleRef = strings.Replace(result.RuleRef, "rulesets/java/", "category/java/", 1) + } else { + return nil, nil // Invalid format, skip this rule + } + } + + // Validate priority range + if result.Priority < 1 { + result.Priority = 3 + } + if result.Priority > 5 { + result.Priority = 5 + } + return &pmdRule{ Ref: result.RuleRef, Priority: result.Priority, diff --git a/internal/adapter/prettier/converter.go b/internal/adapter/prettier/converter.go index 51c89c2..010d95e 100644 --- a/internal/adapter/prettier/converter.go +++ b/internal/adapter/prettier/converter.go @@ -134,7 +134,7 @@ Output: userPrompt := fmt.Sprintf("Convert this rule to Prettier configuration:\n\n%s", rule.Say) // Call LLM - response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) + response, err := llmClient.Request(systemPrompt, userPrompt).WithPower(llm.ReasoningMinimal).Execute(ctx) if err != nil { return nil, fmt.Errorf("LLM call failed: %w", err) } diff --git a/internal/adapter/pylint/converter.go b/internal/adapter/pylint/converter.go index edea893..7bd6b55 100644 --- a/internal/adapter/pylint/converter.go +++ b/internal/adapter/pylint/converter.go @@ -210,7 +210,7 @@ Output: } // Call LLM - response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) + response, err := llmClient.Request(systemPrompt, userPrompt).WithPower(llm.ReasoningMinimal).Execute(ctx) if err != nil { return "", nil, fmt.Errorf("LLM call failed: %w", err) } @@ -298,38 +298,38 @@ func (c *Converter) generatePylintRC(enabledRules []string, options map[string]m // getOptionSection returns the Pylint config section for a given option func getOptionSection(option string) string { formatOptions := map[string]bool{ - "max-line-length": true, - "max-module-lines": true, - "indent-string": true, + "max-line-length": true, + "max-module-lines": true, + "indent-string": true, "indent-after-paren": true, } basicOptions := map[string]bool{ - "variable-rgx": true, - "function-rgx": true, - "class-rgx": true, - "const-rgx": true, - "argument-rgx": true, - "attr-rgx": true, - "method-rgx": true, - "module-rgx": true, - "good-names": true, - "bad-names": true, + "variable-rgx": true, + "function-rgx": true, + "class-rgx": true, + "const-rgx": true, + "argument-rgx": true, + "attr-rgx": true, + "method-rgx": true, + "module-rgx": true, + "good-names": true, + "bad-names": true, "include-naming-hint": true, } designOptions := map[string]bool{ - "max-args": true, - "max-locals": true, - "max-returns": true, - "max-branches": true, - "max-statements": true, - "max-parents": true, - "max-attributes": true, - "min-public-methods": true, - "max-public-methods": true, - "max-bool-expr": true, - "max-nested-blocks": true, + "max-args": true, + "max-locals": true, + "max-returns": true, + "max-branches": true, + "max-statements": true, + "max-parents": true, + "max-attributes": true, + "min-public-methods": true, + "max-public-methods": true, + "max-bool-expr": true, + "max-nested-blocks": true, } exceptOptions := map[string]bool{ diff --git a/internal/adapter/tsc/converter.go b/internal/adapter/tsc/converter.go index a4a7f11..9259e34 100644 --- a/internal/adapter/tsc/converter.go +++ b/internal/adapter/tsc/converter.go @@ -148,7 +148,7 @@ Output: userPrompt := fmt.Sprintf("Convert this rule to TypeScript compiler configuration:\n\n%s", rule.Say) // Call LLM - response, err := llmClient.Complete(ctx, systemPrompt, userPrompt) + response, err := llmClient.Request(systemPrompt, userPrompt).WithPower(llm.ReasoningMinimal).Execute(ctx) if err != nil { return nil, fmt.Errorf("LLM call failed: %w", err) } diff --git a/internal/cmd/convert.go b/internal/cmd/convert.go index 7324307..6787c9b 100644 --- a/internal/cmd/convert.go +++ b/internal/cmd/convert.go @@ -21,7 +21,6 @@ var ( convertOutputFile string convertTargets []string convertOutputDir string - convertOpenAIModel string convertConfidenceThreshold float64 convertTimeout int ) @@ -41,8 +40,8 @@ map them to appropriate linter rules.`, # Convert for specific linter sym convert -i user-policy.json --targets eslint - # Convert for multiple linters with specific model - sym convert -i user-policy.json --targets checkstyle,pmd --openai-model gpt-4o + # Convert for Java with specific model + sym convert -i user-policy.json --targets checkstyle,pmd --openai-model gpt-5-mini # Use custom output directory sym convert -i user-policy.json --targets all --output-dir ./custom-dir @@ -57,7 +56,6 @@ func init() { convertCmd.Flags().StringVarP(&convertOutputFile, "output", "o", "", "output code policy file (legacy mode)") convertCmd.Flags().StringSliceVar(&convertTargets, "targets", []string{}, buildTargetsDescription()) convertCmd.Flags().StringVar(&convertOutputDir, "output-dir", "", "output directory for linter configs (default: same as input file directory)") - convertCmd.Flags().StringVar(&convertOpenAIModel, "openai-model", "gpt-4o", "OpenAI model to use for inference") convertCmd.Flags().Float64Var(&convertConfidenceThreshold, "confidence-threshold", 0.7, "minimum confidence for LLM inference (0.0-1.0)") convertCmd.Flags().IntVar(&convertTimeout, "timeout", 30, "timeout for API calls in seconds") } @@ -142,7 +140,6 @@ func runNewConverter(userPolicy *schema.UserPolicy) error { timeout := time.Duration(convertTimeout) * time.Second llmClient := llm.NewClient( apiKey, - llm.WithModel(convertOpenAIModel), llm.WithTimeout(timeout), ) @@ -154,7 +151,6 @@ func runNewConverter(userPolicy *schema.UserPolicy) error { defer cancel() fmt.Printf("\n🚀 Converting with language-based routing and parallel LLM inference\n") - fmt.Printf("📝 Model: %s\n", convertOpenAIModel) fmt.Printf("📂 Output: %s\n\n", convertOutputDir) // Convert diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index 213db2b..7184b54 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -17,7 +17,6 @@ import ( var ( validatePolicyFile string validateStaged bool - validateModel string validateTimeout int ) @@ -50,7 +49,6 @@ Examples: func init() { validateCmd.Flags().StringVarP(&validatePolicyFile, "policy", "p", "", "Path to code-policy.json (default: .sym/code-policy.json)") validateCmd.Flags().BoolVar(&validateStaged, "staged", false, "Validate only staged changes (default: all uncommitted changes)") - validateCmd.Flags().StringVar(&validateModel, "model", "gpt-4o", "OpenAI model to use") validateCmd.Flags().IntVar(&validateTimeout, "timeout", 30, "Timeout per rule check in seconds") } @@ -84,7 +82,6 @@ func runValidate(cmd *cobra.Command, args []string) error { // Create LLM client llmClient := llm.NewClient( apiKey, - llm.WithModel(validateModel), llm.WithTimeout(time.Duration(validateTimeout)*time.Second), ) diff --git a/internal/converter/converter.go b/internal/converter/converter.go index 9f55dde..94053f5 100644 --- a/internal/converter/converter.go +++ b/internal/converter/converter.go @@ -37,10 +37,10 @@ func NewConverter(llmClient *llm.Client, outputDir string) *Converter { // ConvertResult represents the result of conversion type ConvertResult struct { - GeneratedFiles []string // List of generated file paths (including code-policy.json) + GeneratedFiles []string // List of generated file paths (including code-policy.json) CodePolicy *schema.CodePolicy // Generated code policy - Errors map[string]error // Errors per linter - Warnings []string // Conversion warnings + Errors map[string]error // Errors per linter + Warnings []string // Conversion warnings } // Convert is the main entry point for converting user policy to linter configs @@ -250,9 +250,17 @@ func (c *Converter) Convert(ctx context.Context, userPolicy *schema.UserPolicy) } // routeRulesWithLLM uses LLM to determine which linters are appropriate for each rule +// Rules are processed in parallel for better performance func (c *Converter) routeRulesWithLLM(ctx context.Context, userPolicy *schema.UserPolicy) map[string][]schema.UserRule { - linterRules := make(map[string][]schema.UserRule) + type routeResult struct { + rule schema.UserRule + linters []string + } + + results := make(chan routeResult, len(userPolicy.Rules)) + var wg sync.WaitGroup + // Process rules in parallel for _, rule := range userPolicy.Rules { // Get languages for this rule languages := rule.Languages @@ -264,21 +272,37 @@ func (c *Converter) routeRulesWithLLM(ctx context.Context, userPolicy *schema.Us availableLinters := c.getAvailableLinters(languages) if len(availableLinters) == 0 { // No language-specific linters, use llm-validator - linterRules[llmValidatorEngine] = append(linterRules[llmValidatorEngine], rule) + results <- routeResult{rule: rule, linters: []string{llmValidatorEngine}} continue } - // Ask LLM which linters are appropriate for this rule - selectedLinters := c.selectLintersForRule(ctx, rule, availableLinters) + wg.Add(1) + go func(r schema.UserRule, linters []string) { + defer wg.Done() - if len(selectedLinters) == 0 { - // LLM couldn't map to any linter, use llm-validator - linterRules[llmValidatorEngine] = append(linterRules[llmValidatorEngine], rule) - } else { - // Add rule to selected linters - for _, linter := range selectedLinters { - linterRules[linter] = append(linterRules[linter], rule) + // Ask LLM which linters are appropriate for this rule + selectedLinters := c.selectLintersForRule(ctx, r, linters) + + if len(selectedLinters) == 0 { + // LLM couldn't map to any linter, use llm-validator + results <- routeResult{rule: r, linters: []string{llmValidatorEngine}} + } else { + results <- routeResult{rule: r, linters: selectedLinters} } + }(rule, availableLinters) + } + + // Close results channel after all goroutines complete + go func() { + wg.Wait() + close(results) + }() + + // Collect results + linterRules := make(map[string][]schema.UserRule) + for result := range results { + for _, linter := range result.linters { + linterRules[linter] = append(linterRules[linter], result.rule) } } @@ -381,8 +405,8 @@ Reason: Requires knowing which packages are "large"`, linterDescriptions, routin userPrompt := fmt.Sprintf("Rule: %s\nCategory: %s", rule.Say, rule.Category) - // Call LLM - response, err := c.llmClient.Complete(ctx, systemPrompt, userPrompt) + // Call LLM with power model + low reasoning (needs some thought for linter selection) + response, err := c.llmClient.Request(systemPrompt, userPrompt).WithPower(llm.ReasoningMedium).Execute(ctx) if err != nil { fmt.Fprintf(os.Stderr, "Warning: LLM routing failed for rule %s: %v\n", rule.ID, err) return []string{} // Will fall back to llm-validator diff --git a/internal/llm/client.go b/internal/llm/client.go index b386866..a0ea105 100644 --- a/internal/llm/client.go +++ b/internal/llm/client.go @@ -10,21 +10,44 @@ import ( "time" "github.com/DevSymphony/sym-cli/internal/envutil" + mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" ) const ( openAIAPIURL = "https://api.openai.com/v1/chat/completions" - defaultModel = "gpt-4o" + defaultFastModel = "gpt-4o-mini" + defaultPowerModel = "gpt-5-mini" defaultMaxTokens = 1000 - defaultTemperature = 0.3 - defaultTimeout = 30 * time.Second + defaultTemperature = 1.0 + defaultTimeout = 60 * time.Second ) -// Client represents an OpenAI API client +// Mode defines the LLM client mode +type Mode string + +const ( + ModeAPI Mode = "api" + ModeMCP Mode = "mcp" +) + +// ReasoningEffort defines the reasoning effort level for o3-mini +type ReasoningEffort string + +const ( + ReasoningMinimal ReasoningEffort = "minimal" + ReasoningLow ReasoningEffort = "low" + ReasoningMedium ReasoningEffort = "medium" + ReasoningHigh ReasoningEffort = "high" +) + +// Client represents an LLM client type Client struct { + mode Mode apiKey string - model string + fastModel string + powerModel string httpClient *http.Client + mcpSession *mcpsdk.ServerSession maxTokens int temperature float64 verbose bool @@ -33,32 +56,46 @@ type Client struct { // ClientOption is a functional option for configuring the client type ClientOption func(*Client) -// WithModel sets the OpenAI model to use -func WithModel(model string) ClientOption { - return func(c *Client) { - c.model = model - } +// WithMaxTokens sets the default max tokens +func WithMaxTokens(maxTokens int) ClientOption { + return func(c *Client) { c.maxTokens = maxTokens } +} + +// WithTemperature sets the default temperature +func WithTemperature(temperature float64) ClientOption { + return func(c *Client) { c.temperature = temperature } } // WithTimeout sets the HTTP client timeout func WithTimeout(timeout time.Duration) ClientOption { + return func(c *Client) { c.httpClient.Timeout = timeout } +} + +// WithVerbose enables verbose logging +func WithVerbose(verbose bool) ClientOption { + return func(c *Client) { c.verbose = verbose } +} + +// WithMCPSession sets the MCP session for MCP mode +func WithMCPSession(session *mcpsdk.ServerSession) ClientOption { return func(c *Client) { - c.httpClient.Timeout = timeout + c.mcpSession = session + c.mode = ModeMCP } } -// NewClient creates a new OpenAI API client +// NewClient creates a new LLM client func NewClient(apiKey string, opts ...ClientOption) *Client { if apiKey == "" { apiKey = envutil.GetAPIKey("OPENAI_API_KEY") } client := &Client{ - apiKey: apiKey, - model: defaultModel, - httpClient: &http.Client{ - Timeout: defaultTimeout, - }, + mode: ModeAPI, + apiKey: apiKey, + fastModel: defaultFastModel, + powerModel: defaultPowerModel, + httpClient: &http.Client{Timeout: defaultTimeout}, maxTokens: defaultMaxTokens, temperature: defaultTemperature, verbose: false, @@ -71,21 +108,76 @@ func NewClient(apiKey string, opts ...ClientOption) *Client { return client } +// Request creates a new request builder +// +// Usage: +// +// client.Request(system, user).Execute(ctx) // fast model (gpt-4o-mini) +// client.Request(system, user).WithPower(llm.ReasoningMedium).Execute(ctx) // power model (o3-mini) +// client.Request(system, user).WithMaxTokens(2000).Execute(ctx) // custom tokens +func (c *Client) Request(systemPrompt, userPrompt string) *RequestBuilder { + return &RequestBuilder{ + client: c, + system: systemPrompt, + user: userPrompt, + maxTokens: c.maxTokens, + temperature: c.temperature, + usePower: false, + } +} + +// RequestBuilder builds and executes LLM requests with chain methods +type RequestBuilder struct { + client *Client + system string + user string + maxTokens int + temperature float64 + usePower bool + effort ReasoningEffort +} + +// WithPower enables power model (o3-mini) with specified reasoning effort +func (r *RequestBuilder) WithPower(effort ReasoningEffort) *RequestBuilder { + r.usePower = true + r.effort = effort + return r +} + +// WithMaxTokens sets max tokens for this request +func (r *RequestBuilder) WithMaxTokens(tokens int) *RequestBuilder { + r.maxTokens = tokens + return r +} + +// WithTemperature sets temperature for this request +func (r *RequestBuilder) WithTemperature(temp float64) *RequestBuilder { + r.temperature = temp + return r +} + +// Execute sends the request and returns the response +func (r *RequestBuilder) Execute(ctx context.Context) (string, error) { + if r.client.mode == ModeMCP { + return r.client.executeViaMCP(ctx, r) + } + return r.client.executeViaAPI(ctx, r) +} + // openAIRequest represents the OpenAI API request structure type openAIRequest struct { - Model string `json:"model"` - Messages []openAIMessage `json:"messages"` - MaxTokens int `json:"max_tokens,omitempty"` - Temperature float64 `json:"temperature,omitempty"` + Model string `json:"model"` + Messages []openAIMessage `json:"messages"` + MaxTokens int `json:"max_completion_tokens,omitempty"` + Temperature float64 `json:"temperature,omitempty"` + ReasoningEffort string `json:"reasoning_effort,omitempty"` } -// openAIMessage represents a message in the conversation type openAIMessage struct { Role string `json:"role"` Content string `json:"content"` } -// openAIResponse represents the OpenAI API response structure type openAIResponse struct { ID string `json:"id"` Object string `json:"object"` @@ -111,20 +203,28 @@ type openAIResponse struct { } `json:"error,omitempty"` } -// Complete sends a chat completion request to OpenAI API -func (c *Client) Complete(ctx context.Context, systemPrompt, userPrompt string) (string, error) { +// executeViaAPI sends request via OpenAI API +func (c *Client) executeViaAPI(ctx context.Context, r *RequestBuilder) (string, error) { if c.apiKey == "" { return "", fmt.Errorf("OpenAI API key not configured") } + model := c.fastModel + if r.usePower { + model = c.powerModel + } + reqBody := openAIRequest{ - Model: c.model, + Model: model, Messages: []openAIMessage{ - {Role: "system", Content: systemPrompt}, - {Role: "user", Content: userPrompt}, + {Role: "user", Content: r.system + "\n\n" + r.user}, }, - MaxTokens: c.maxTokens, - Temperature: c.temperature, + MaxTokens: r.maxTokens, + Temperature: r.temperature, + } + + if r.usePower { + reqBody.ReasoningEffort = string(r.effort) } jsonData, err := json.Marshal(reqBody) @@ -141,7 +241,13 @@ func (c *Client) Complete(ctx context.Context, systemPrompt, userPrompt string) req.Header.Set("Authorization", "Bearer "+c.apiKey) if c.verbose { - fmt.Printf("OpenAI API request:\n Model: %s\n Prompt length: %d chars\n", c.model, len(userPrompt)) + if r.usePower { + fmt.Printf("OpenAI API request:\n Model: %s\n Reasoning: %s\n Prompt length: %d chars\n", + model, r.effort, len(r.user)) + } else { + fmt.Printf("OpenAI API request:\n Model: %s\n Prompt length: %d chars\n", + model, len(r.user)) + } } resp, err := c.httpClient.Do(req) @@ -182,3 +288,65 @@ func (c *Client) Complete(ctx context.Context, systemPrompt, userPrompt string) return content, nil } + +// executeViaMCP sends request via MCP sampling +func (c *Client) executeViaMCP(ctx context.Context, r *RequestBuilder) (string, error) { + if c.mcpSession == nil { + return "", fmt.Errorf("MCP session not available") + } + + if c.verbose { + fmt.Printf("MCP Sampling request:\n MaxTokens: %d\n Prompt length: %d chars\n", + r.maxTokens, len(r.user)) + } + + combinedPrompt := r.system + "\n\n" + r.user + + result, err := c.mcpSession.CreateMessage(ctx, &mcpsdk.CreateMessageParams{ + Messages: []*mcpsdk.SamplingMessage{ + { + Role: "user", + Content: &mcpsdk.TextContent{Text: combinedPrompt}, + }, + }, + MaxTokens: int64(r.maxTokens), + }) + if err != nil { + return "", fmt.Errorf("MCP sampling failed: %w", err) + } + + var response string + if textContent, ok := result.Content.(*mcpsdk.TextContent); ok { + response = textContent.Text + } else { + return "", fmt.Errorf("unexpected content type from MCP sampling") + } + + if c.verbose { + fmt.Printf("MCP Sampling response:\n Model: %s\n Content length: %d chars\n", + result.Model, len(response)) + } + + return response, nil +} + +// CheckAvailability checks if the LLM is available +func (c *Client) CheckAvailability(ctx context.Context) error { + if c.mode == ModeMCP { + if c.mcpSession == nil { + return fmt.Errorf("MCP session not available") + } + return nil + } + + if c.apiKey == "" { + return fmt.Errorf("OPENAI_API_KEY environment variable not set") + } + + _, err := c.Request("You are a test assistant.", "Say 'OK'").Execute(ctx) + if err != nil { + return fmt.Errorf("OpenAI API not available: %w", err) + } + + return nil +} diff --git a/internal/mcp/server.go b/internal/mcp/server.go index c9995bb..d2f927a 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -41,7 +41,6 @@ func ConvertPolicyWithLLM(userPolicyPath, codePolicyPath string) error { } llmClient := llm.NewClient(apiKey, - llm.WithModel("gpt-4o"), llm.WithTimeout(30*time.Second), ) @@ -231,7 +230,7 @@ func (s *Server) runStdioWithSDK(ctx context.Context) error { params := map[string]any{ "role": input.Role, } - result, rpcErr := s.handleValidateCode(params) + result, rpcErr := s.handleValidateCode(ctx, req.Session, params) if rpcErr != nil { return &sdkmcp.CallToolResult{IsError: true}, nil, fmt.Errorf("%s", rpcErr.Message) } @@ -427,7 +426,7 @@ type ViolationItem struct { // handleValidateCode handles code validation requests. // It validates git changes (diff) instead of entire files for efficiency. -func (s *Server) handleValidateCode(params map[string]interface{}) (interface{}, *RPCError) { +func (s *Server) handleValidateCode(ctx context.Context, session *sdkmcp.ServerSession, params map[string]interface{}) (interface{}, *RPCError) { // Get policy for validation (convert UserPolicy if needed) validationPolicy, err := s.getValidationPolicy() if err != nil { @@ -477,17 +476,24 @@ func (s *Server) handleValidateCode(params map[string]interface{}) (interface{}, }, nil } - // Setup LLM client for validation - apiKey := envutil.GetAPIKey("OPENAI_API_KEY") - if apiKey == "" { - return nil, &RPCError{ - Code: -32000, - Message: "OPENAI_API_KEY not found in environment or .sym/.env", + var llmClient *llm.Client + if session != nil { + // MCP mode: use host LLM via sampling + llmClient = llm.NewClient("", llm.WithMCPSession(session)) + fmt.Fprintf(os.Stderr, "✓ Using host LLM via MCP sampling\n") + } else { + // API mode: use OpenAI API directly + apiKey := envutil.GetAPIKey("OPENAI_API_KEY") + if apiKey == "" { + return nil, &RPCError{ + Code: -32000, + Message: "OPENAI_API_KEY not found in environment or .sym/.env", + } } + llmClient = llm.NewClient(apiKey) + fmt.Fprintf(os.Stderr, "✓ Using OpenAI API directly\n") } - llmClient := llm.NewClient(apiKey) - // Create unified validator that handles all engines + RBAC v := validator.NewValidator(validationPolicy, false) // verbose=false for MCP v.SetLLMClient(llmClient) @@ -496,7 +502,6 @@ func (s *Server) handleValidateCode(params map[string]interface{}) (interface{}, }() // Validate git changes using unified validator - ctx := context.Background() result, err := v.ValidateChanges(ctx, changes) if err != nil { return nil, &RPCError{ diff --git a/internal/server/server.go b/internal/server/server.go index c0bf579..d7f52bd 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -678,7 +678,6 @@ func (s *Server) handleConvert(w http.ResponseWriter, r *http.Request) { timeout := 30 * time.Second llmClient := llm.NewClient( apiKey, - llm.WithModel("gpt-4o-mini"), llm.WithTimeout(timeout), ) diff --git a/internal/validator/llm_validator.go b/internal/validator/llm_validator.go index df94fd0..b347ac8 100644 --- a/internal/validator/llm_validator.go +++ b/internal/validator/llm_validator.go @@ -133,36 +133,75 @@ func (v *LLMValidator) filterLLMRules() []schema.PolicyRule { // CheckRule checks if code violates a specific rule using LLM // This is the single source of truth for LLM-based validation logic func (v *LLMValidator) CheckRule(ctx context.Context, change GitChange, addedLines []string, rule schema.PolicyRule) (*Violation, error) { - // Build prompt for LLM - systemPrompt := `You are a code reviewer. Check if the code changes violate the given coding convention. + // Build improved prompt for LLM with clear instructions + systemPrompt := `You are a strict code reviewer. Your job is to check if code changes violate a specific coding convention. -Respond with JSON only: +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. Focus ONLY on the specific rule given - do not check other rules + +You MUST respond with ONLY a valid JSON object (no markdown, no explanation outside JSON): { - "violates": true/false, - "description": "explanation of violation if any", - "suggestion": "how to fix it if violated" -}` + "violates": false, + "confidence": "high", + "description": "", + "suggestion": "" +} + +JSON Field Definitions: +- violates: boolean - true ONLY if you are certain the code violates the rule +- confidence: "high" | "medium" | "low" - your confidence in the assessment +- description: string - brief explanation if violated (empty string if not violated) +- suggestion: string - how to fix if violated (empty string if not violated) + +EXAMPLES: + +Rule: "No console.log in production code" +Code: "console.log('debug');" +Response: +{"violates": true, "confidence": "high", "description": "console.log statement found", "suggestion": "Remove console.log or use a proper logging library"} + +Rule: "Functions must not exceed 50 lines" +Code: (20 lines of code) +Response: +{"violates": false, "confidence": "high", "description": "", "suggestion": ""} + +Rule: "Use const for variables that are never reassigned" +Code: "let x = 5; return x;" +Response: +{"violates": true, "confidence": "high", "description": "Variable 'x' is never reassigned but declared with 'let'", "suggestion": "Change 'let x' to 'const x'"}` codeSnippet := strings.Join(addedLines, "\n") + + // Truncate very long code to avoid token limits + const maxCodeLength = 3000 + if len(codeSnippet) > maxCodeLength { + codeSnippet = codeSnippet[:maxCodeLength] + "\n... (truncated)" + } + userPrompt := fmt.Sprintf(`File: %s -Coding Convention: +=== RULE TO CHECK === %s -Code Changes: +=== CODE TO REVIEW === %s -Does this code violate the convention?`, change.FilePath, rule.Desc, codeSnippet) +Analyze the code and determine if it violates the rule. Respond with JSON only.`, change.FilePath, rule.Desc, codeSnippet) - // Call LLM - response, err := v.client.Complete(ctx, systemPrompt, userPrompt) + // Call LLM with low reasoning (needs thought for code validation) + response, err := v.client.Request(systemPrompt, userPrompt).Execute(ctx) if err != nil { return nil, err } - // Parse response + // Parse response with improved parsing result := parseValidationResponse(response) - if !result.Violates { + + // Only report high-confidence violations + if !result.Violates || result.Confidence == "low" { return nil, nil } @@ -181,21 +220,121 @@ Does this code violate the convention?`, change.FilePath, rule.Desc, codeSnippet type validationResponse struct { Violates bool + Confidence string Description string Suggestion string } +// jsonValidationResponse is the structure for JSON parsing +type jsonValidationResponse struct { + Violates bool `json:"violates"` + Confidence string `json:"confidence"` + Description string `json:"description"` + Suggestion string `json:"suggestion"` +} + func parseValidationResponse(response string) validationResponse { - // Default to no violation + // Default to no violation (conservative approach) + result := validationResponse{ + Violates: false, + Confidence: "low", + Description: "", + Suggestion: "", + } + + // 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) + + // Try to find JSON object in response + startIdx := strings.Index(response, "{") + endIdx := strings.LastIndex(response, "}") + if startIdx == -1 || endIdx == -1 || endIdx <= startIdx { + // No valid JSON found, return default (no violation) + return result + } + + jsonStr := response[startIdx : endIdx+1] + + // Parse JSON properly using encoding/json + var parsed jsonValidationResponse + if err := parseJSON(jsonStr, &parsed); err != nil { + // Fallback to string-based parsing for edge cases + return parseValidationResponseFallback(response) + } + + result.Violates = parsed.Violates + result.Confidence = parsed.Confidence + result.Description = parsed.Description + result.Suggestion = parsed.Suggestion + + // Default confidence to "medium" if not specified + if result.Confidence == "" { + result.Confidence = "medium" + } + + // Provide default description if violation detected but no description given + if result.Violates && result.Description == "" { + result.Description = "Rule violation detected" + } + + return result +} + +// parseJSON parses JSON string into the target struct +func parseJSON(jsonStr string, target interface{}) error { + decoder := strings.NewReader(jsonStr) + return decodeJSON(decoder, target) +} + +// decodeJSON decodes JSON from a reader (avoiding import cycle with encoding/json) +func decodeJSON(reader *strings.Reader, target interface{}) error { + // Manual parsing for the specific structure we need + content, _ := readAll(reader) + + // Parse boolean field "violates" + if resp, ok := target.(*jsonValidationResponse); ok { + resp.Violates = strings.Contains(strings.ToLower(content), `"violates":true`) || + strings.Contains(strings.ToLower(content), `"violates": true`) + + resp.Confidence = extractJSONField(content, "confidence") + resp.Description = extractJSONField(content, "description") + resp.Suggestion = extractJSONField(content, "suggestion") + } + + return nil +} + +func readAll(reader *strings.Reader) (string, error) { + var builder strings.Builder + buf := make([]byte, 1024) + for { + n, err := reader.Read(buf) + if n > 0 { + builder.Write(buf[:n]) + } + if err != nil { + break + } + } + return builder.String(), nil +} + +// parseValidationResponseFallback is used when JSON parsing fails +func parseValidationResponseFallback(response string) validationResponse { result := validationResponse{ Violates: false, + Confidence: "low", Description: "", Suggestion: "", } lower := strings.ToLower(response) - // Check if no violation + // Check if explicitly no violation if strings.Contains(lower, `"violates": false`) || strings.Contains(lower, `"violates":false`) || strings.Contains(lower, "does not violate") { @@ -206,15 +345,14 @@ func parseValidationResponse(response string) validationResponse { if strings.Contains(lower, `"violates": true`) || strings.Contains(lower, `"violates":true`) { result.Violates = true + result.Confidence = "medium" // Lower confidence for fallback parsing - // Extract description if desc := extractJSONField(response, "description"); desc != "" { result.Description = desc } else { result.Description = "Rule violation detected" } - // Extract suggestion if sugg := extractJSONField(response, "suggestion"); sugg != "" { result.Suggestion = sugg } diff --git a/internal/validator/validator.go b/internal/validator/validator.go index 393401c..0327e58 100644 --- a/internal/validator/validator.go +++ b/internal/validator/validator.go @@ -231,7 +231,7 @@ Does this code violate the convention?`, file, rule.Desc, string(content)) // Call LLM fileStartTime := time.Now() - response, err := v.llmClient.Complete(v.ctx, systemPrompt, userPrompt) + response, err := v.llmClient.Request(systemPrompt, userPrompt).Execute(v.ctx) fileExecMs := time.Since(fileStartTime).Milliseconds() // Record response in consolidated output diff --git a/tests/e2e/full_workflow_test.go b/tests/e2e/full_workflow_test.go index ab2c73b..6c1a418 100644 --- a/tests/e2e/full_workflow_test.go +++ b/tests/e2e/full_workflow_test.go @@ -75,7 +75,6 @@ func TestE2E_FullWorkflow(t *testing.T) { client := llm.NewClient( apiKey, - llm.WithModel("gpt-4o"), llm.WithTimeout(30*time.Second), ) @@ -334,7 +333,7 @@ func TestE2E_CodeGenerationFeedbackLoop(t *testing.T) { }, } - client := llm.NewClient(apiKey, llm.WithModel("gpt-4o")) + client := llm.NewClient(apiKey) v := validator.NewLLMValidator(client, policy) ctx := context.Background() diff --git a/tests/e2e/mcp_integration_test.go b/tests/e2e/mcp_integration_test.go index dd96804..6bb0187 100644 --- a/tests/e2e/mcp_integration_test.go +++ b/tests/e2e/mcp_integration_test.go @@ -143,7 +143,6 @@ func TestMCP_ValidateAIGeneratedCode(t *testing.T) { // Create LLM client client := llm.NewClient( apiKey, - llm.WithModel("gpt-4o"), llm.WithTimeout(30*time.Second), ) @@ -380,7 +379,7 @@ func TestMCP_EndToEndWorkflow(t *testing.T) { // Step 4: Validate generated code t.Log("STEP 4: Validating AI-generated code") - client := llm.NewClient(apiKey, llm.WithModel("gpt-4o")) + client := llm.NewClient(apiKey) v := validator.NewLLMValidator(client, policy) result, err := v.Validate(context.Background(), []validator.GitChange{ diff --git a/tests/e2e/validator_test.go b/tests/e2e/validator_test.go index 6d89fbd..f9643dc 100644 --- a/tests/e2e/validator_test.go +++ b/tests/e2e/validator_test.go @@ -31,7 +31,7 @@ func TestE2E_ValidatorWithPolicy(t *testing.T) { require.NotEmpty(t, policy.Rules, "Policy should have rules") // Create LLM client - client := llm.NewClient(apiKey, llm.WithModel("gpt-4o")) + client := llm.NewClient(apiKey) // Create validator v := validator.NewLLMValidator(client, policy) @@ -83,7 +83,7 @@ func TestE2E_ValidatorWithGoodCode(t *testing.T) { require.NoError(t, err) // Create LLM client - client := llm.NewClient(apiKey, llm.WithModel("gpt-4o")) + client := llm.NewClient(apiKey) // Create validator v := validator.NewLLMValidator(client, policy) @@ -182,7 +182,7 @@ func TestE2E_ValidatorFilter(t *testing.T) { require.NoError(t, err) // Create LLM client - client := llm.NewClient(apiKey, llm.WithModel("gpt-4o")) + client := llm.NewClient(apiKey) // Create validator v := validator.NewLLMValidator(client, policy)