diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md deleted file mode 100644 index 5edd148..0000000 --- a/.github/copilot-instructions.md +++ /dev/null @@ -1,70 +0,0 @@ -# Copilot Instructions for cl-llm - -## Project Overview - -LLM-powered code review system with multiple integration modes: -- **Terminal CLI** (`review.py`) - Direct command-line usage -- **Git hooks** - Pre-commit/pre-push automation -- **GitHub Actions** - CI/CD integration with PR comments - -Uses OpenAI-compatible API against configurable endpoints. - -## Environment Setup - -```bash -# Activate Python virtual environment -source .python-venv/bin/activate.fish - -# Set API key -export LLM_API_KEY="your-api-key" - -# Optional: Override base URL and model -export LLM_BASE_URL="https://api.opencode.ai/v1" -export LLM_MODEL="anthropic/claude-sonnet-4" - -# Optional: Increase timeout for large reviews (default: 180 seconds) -export LLM_TIMEOUT="300" -``` - -## Code Patterns - -### API Client Configuration - -Configuration is loaded from `review_config.json`, but environment variables take precedence: - -```python -from config import ReviewConfig - -config = ReviewConfig() - -# These methods check env vars first, then fall back to config file -api_key = config.get_api_key() # LLM_API_KEY env var -base_url = config.get_base_url() # LLM_BASE_URL env var -model = config.get_model() # LLM_MODEL env var -timeout = config.get_timeout() # LLM_TIMEOUT env var -``` - -### Making LLM Requests - -The system uses OpenAI-compatible client: - -```python -from openai import OpenAI - -client = OpenAI( - base_url=config.get_base_url, - timeout=config.get_timeout()(), - api_key=config.get_api_key() -) - -response = client.chat.completions.create( - model=config.get_model(), - messages=[{"role": "user", "content": "Your prompt"}] -) -``` - -## Important Notes - -- **Never commit API keys**: Use environment variables -- **All code comments must be in English** -- **Shell**: Project uses fish shell for environment configuration diff --git a/CHANGELOG.md b/CHANGELOG.md index 09ce59d..7ac9429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ ## [Unreleased] +### Added +- `max_tokens` input in GitHub Action to override max tokens per LLM request per workflow run. +- `timeout` input in GitHub Action to override LLM request timeout per workflow run. +- `LLM_MAX_RESPONSE_TOKENS` environment variable to control max tokens in LLM response (default: 16384). +- `llm.max_response_tokens` config key (default: 16384). + +### Changed +- Default `llm.timeout` increased from 180 to 600 seconds to accommodate slow providers and large diffs with chunking. +- Default `max_tokens_per_request` increased to 32768 (see v0.1.3 notes). +- LLM requests now use `system` + `user` message roles instead of a single `user` message, giving reviewer instructions higher priority in the model's context window. +- `max_tokens` is now explicitly passed in every LLM API call, preventing unbounded response generation. + +## [0.1.3] - 2026-03-27 + ### Added - `--context N` CLI flag to override the number of diff context lines per run (e.g. `--context 20`). - `context` input in GitHub Action to override context lines per workflow run. @@ -27,8 +41,9 @@ ### Changed - Default `output.max_context_lines` increased from 3 to 10 to reduce false positives caused by partially visible try/except blocks and multiline function calls. -- Default `max_tokens_per_request` increased from 4096 to 8192 to handle larger diffs that result - from wider context windows. +- Default `max_tokens_per_request` increased from 4096 to 32768 to handle large PRs without + chunking on modern LLMs with wide context windows (e.g. Claude Sonnet 4 supports 200K tokens). + Override with `LLM_MAX_TOKENS_PER_REQUEST` environment variable if a lower limit is needed. - Prompt now explicitly tells the LLM how many context lines are included and instructs it not to flag constructs (try/except, multiline calls, class definitions) that extend beyond the visible area. - Tightened LLM prompt to reduce noise: requires file:line references, prohibits generic advice, diff --git a/CLAUDE.md b/CLAUDE.md index c8e90db..ad5fcdf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -18,7 +18,7 @@ Python, JavaScript, TypeScript, JSX, TSX, Java, C, C++, Go ```bash # Activate virtual environment -source .python-venv/bin/activate.fish +source .venv/bin/activate.fish # Load API key (fish shell) source .env.fish @@ -35,14 +35,21 @@ python review.py --format json # JSON output for CI/CD python review.py --strict # Block on warnings too python review.py --verbose # Verbose output python review.py --test-connection # Test LLM API connectivity +python review.py --mode staged --offline # Static analysis only (no LLM) +python review.py --mode staged --context 20 # Override context lines +python review.py --mode staged --trace-llm # Debug: print full LLM request/response # Install git hooks python install_hooks.py # Code formatting and linting -ruff format . # Format all Python files -ruff check . # Lint all Python files -ruff check --fix . # Auto-fix linting issues +.venv/bin/ruff format . # Format all Python files +.venv/bin/ruff check . # Lint all Python files +.venv/bin/ruff check --fix . # Auto-fix linting issues + +# Tests +pytest test_docstrings.py -v # Test docstring detection +pytest test_code_suggestions.py -v # Test code suggestion parsing # Monitoring python monitor.py health # System health check @@ -86,9 +93,11 @@ static_analyzer.py Fallback security analysis when LLM unavailable - `LLM_API_KEY` - API key for LLM service - `LLM_BASE_URL` - Override base URL (optional) - `LLM_MODEL` - Override model name (optional) -- `LLM_TIMEOUT` - Request timeout in seconds (default: 180) -- `LLM_MAX_TOKENS_PER_REQUEST` - Max tokens per review chunk (default: 4096) +- `LLM_TIMEOUT` - Request timeout in seconds (default: 600) +- `LLM_MAX_TOKENS_PER_REQUEST` - Max input tokens per review chunk (default: 32768) +- `LLM_MAX_RESPONSE_TOKENS` - Max tokens in LLM response (default: 16384) - `LLM_TOKEN_LIMIT_STRATEGY` - Strategy when exceeding tokens: `chunk`, `truncate`, or `skip` (default: `chunk`) +- `LLM_CODE_SUGGESTIONS` - Enable inline code suggestions: `true` or `false` (default: `false`) ### Current Setup - Model: `anthropic/claude-sonnet-4` diff --git a/README.md b/README.md index f0abd69..db0f4db 100644 --- a/README.md +++ b/README.md @@ -124,8 +124,9 @@ Edit `review_config.json` to customize rules, or use environment variables: | `LLM_API_KEY` | API key for LLM service | Yes | | `LLM_BASE_URL` | API endpoint URL | No (uses config) | | `LLM_MODEL` | Model name | No (uses config) | -| `LLM_TIMEOUT` | Request timeout in seconds (default: 180) | No | -| `LLM_MAX_TOKENS_PER_REQUEST` | Max tokens per review chunk (default: 8192) | No | +| `LLM_TIMEOUT` | Request timeout in seconds (default: 600) | No | +| `LLM_MAX_TOKENS_PER_REQUEST` | Max input tokens per review chunk (default: 32768) | No | +| `LLM_MAX_RESPONSE_TOKENS` | Max tokens in LLM response (default: 16384) | No | | `LLM_TOKEN_LIMIT_STRATEGY` | Strategy when exceeding tokens: `chunk`, `truncate`, or `skip` (default: `chunk`) | No | | `LLM_CODE_SUGGESTIONS` | Enable inline code change suggestions: `true` or `false` (default: `false`) | No | @@ -191,7 +192,7 @@ For large diffs that exceed LLM token limits, the system automatically splits th ```json { "llm": { - "max_tokens_per_request": 8192, + "max_tokens_per_request": 32768, "token_limit_strategy": "chunk", "chars_per_token": 4 } @@ -199,7 +200,7 @@ For large diffs that exceed LLM token limits, the system automatically splits th ``` **Options:** -- `max_tokens_per_request` - Maximum tokens per LLM request (default: 8192) +- `max_tokens_per_request` - Maximum tokens per LLM request (default: 32768) - `token_limit_strategy` - Strategy for large diffs: `"chunk"` (split and review parts) or `"truncate"` (review only the beginning) - `chars_per_token` - Character to token ratio for estimation (default: 4) @@ -329,6 +330,8 @@ Add optional inputs to customize behavior: fail_on_critical: 'true' # Fail action on critical issues (default: true) inline_comments: 'true' # Post inline comments on code lines (default: true) code_suggestions: 'true' # Enable code change suggestions (default: true) + # max_tokens: '32768' # Max tokens per LLM request (default: 32768) + # timeout: '600' # LLM request timeout in seconds (default: 600) - name: Check results if: always() @@ -351,6 +354,8 @@ Add optional inputs to customize behavior: | `fail_on_critical` | Fail on critical issues | No | `true` | | `inline_comments` | Post inline review comments on code lines | No | `true` | | `code_suggestions` | Enable inline code change suggestions | No | `true` | +| `max_tokens` | Max tokens per LLM request | No | `32768` | +| `timeout` | LLM request timeout in seconds | No | `600` | | `context` | Number of context lines around each change | No | `10` | ### Action Outputs diff --git a/VERSION b/VERSION new file mode 100644 index 0000000..39b8996 --- /dev/null +++ b/VERSION @@ -0,0 +1,2 @@ +0.1.3 + diff --git a/action.yml b/action.yml index 5d99193..ca02d4e 100644 --- a/action.yml +++ b/action.yml @@ -45,6 +45,12 @@ inputs: description: 'Enable inline code change suggestions (default: true)' required: false default: 'true' + max_tokens: + description: 'Max tokens per LLM request (default: 32768). Reduce if your provider has a lower limit.' + required: false + timeout: + description: 'LLM request timeout in seconds (default: 600). Increase for slow providers or large diffs.' + required: false context: description: 'Number of context lines around each change (default: 10)' required: false @@ -110,6 +116,8 @@ runs: LLM_BASE_URL: ${{ inputs.base_url }} LLM_MODEL: ${{ inputs.model }} LLM_CODE_SUGGESTIONS: ${{ inputs.code_suggestions }} + LLM_MAX_TOKENS_PER_REQUEST: ${{ inputs.max_tokens }} + LLM_TIMEOUT: ${{ inputs.timeout }} run: | RESULT_FILE="${{ runner.temp }}/llm-review-result.json" diff --git a/config.py b/config.py index 68378b1..a3244cd 100644 --- a/config.py +++ b/config.py @@ -19,9 +19,10 @@ class ReviewConfig: "model": "gpt-oss:20b", "base_url": None, "api_key_env": "LLM_API_KEY", - "timeout": 180, + "timeout": 600, "max_retries": 3, - "max_tokens_per_request": 8192, + "max_tokens_per_request": 32768, + "max_response_tokens": 16384, "token_limit_strategy": "chunk", # "truncate", "chunk", or "skip" "chars_per_token": 4, # Character-to-token ratio for estimation }, @@ -173,26 +174,34 @@ def get_model(self) -> str: logger.debug("Using model from config: %s", config_model) return config_model - def get_max_tokens(self) -> int: - """Get max tokens per request. Environment variable takes precedence.""" - env_value = os.getenv("LLM_MAX_TOKENS_PER_REQUEST") + def _get_int_config(self, env_var: str, config_key: str) -> int: + """Get integer value from environment variable, falling back to config.""" + env_value = os.getenv(env_var) if env_value: try: value = int(env_value) - logger.debug( - "Using max tokens from LLM_MAX_TOKENS_PER_REQUEST: %d", value - ) + logger.debug("Using %s from %s: %d", config_key, env_var, value) return value except ValueError: logger.warning( - "Invalid LLM_MAX_TOKENS_PER_REQUEST value: %s. Using config.", - env_value, + "Invalid %s value: %s. Using config.", env_var, env_value ) - - config_value = self.get("llm.max_tokens_per_request", 4096) - logger.debug("Using max tokens from config: %d", config_value) + config_value = self.get(config_key) + logger.debug("Using %s from config: %d", config_key, config_value) return config_value + def get_max_tokens(self) -> int: + """Get max input tokens per request. Environment variable takes precedence.""" + return self._get_int_config( + "LLM_MAX_TOKENS_PER_REQUEST", "llm.max_tokens_per_request" + ) + + def get_max_response_tokens(self) -> int: + """Get max response tokens for LLM output. Environment variable takes precedence.""" + return self._get_int_config( + "LLM_MAX_RESPONSE_TOKENS", "llm.max_response_tokens" + ) + def get_token_limit_strategy(self) -> str: """Get token limit strategy. Environment variable takes precedence.""" env_value = os.getenv("LLM_TOKEN_LIMIT_STRATEGY") @@ -212,7 +221,7 @@ def get_token_limit_strategy(self) -> str: valid_strategies, ) - config_value = self.get("llm.token_limit_strategy", "chunk") + config_value = self.get("llm.token_limit_strategy") if config_value not in valid_strategies: logger.warning( "Invalid token_limit_strategy in config: %s. Using 'chunk'.", @@ -228,11 +237,11 @@ def get_code_suggestions_enabled(self) -> bool: env_value = os.getenv("LLM_CODE_SUGGESTIONS") if env_value is not None: return env_value.lower() in ("true", "1", "yes") - return bool(self.get("review.enable_code_suggestions", False)) + return bool(self.get("review.enable_code_suggestions")) def get_chars_per_token(self) -> int: """Get character-to-token ratio for estimation.""" - config_value = self.get("llm.chars_per_token", 4) + config_value = self.get("llm.chars_per_token") if config_value < 1: logger.warning("chars_per_token must be >= 1. Using default 4.") return 4 @@ -241,20 +250,7 @@ def get_chars_per_token(self) -> int: def get_timeout(self) -> int: """Get LLM request timeout in seconds. Environment variable takes precedence.""" - env_value = os.getenv("LLM_TIMEOUT") - if env_value: - try: - value = int(env_value) - logger.debug("Using timeout from LLM_TIMEOUT: %d", value) - return value - except ValueError: - logger.warning( - "Invalid LLM_TIMEOUT value: %s. Using config.", env_value - ) - - config_value = self.get("llm.timeout", 180) - logger.debug("Using timeout from config: %d", config_value) - return config_value + return self._get_int_config("LLM_TIMEOUT", "llm.timeout") def is_file_supported(self, file_path: str) -> bool: """Check if file extension is supported for review.""" diff --git a/examples/workflow-advanced.yml b/examples/workflow-advanced.yml index 624c933..92e1e89 100644 --- a/examples/workflow-advanced.yml +++ b/examples/workflow-advanced.yml @@ -46,6 +46,14 @@ jobs: # Enable inline code change suggestions (default: true) code_suggestions: 'true' + # Max tokens per LLM request (default: 32768 — covers most PRs in a single request) + # Reduce this value if your LLM provider has a lower token limit per request + # max_tokens: '32768' + + # LLM request timeout in seconds (default: 600) + # Increase for slow providers or when reviewing large diffs with chunking + # timeout: '300' + - name: Review Summary if: always() run: | diff --git a/examples/workflow-basic.yml b/examples/workflow-basic.yml index 26d3619..f36bf26 100644 --- a/examples/workflow-basic.yml +++ b/examples/workflow-basic.yml @@ -34,3 +34,7 @@ jobs: api_key: ${{ secrets.LLM_API_KEY }} base_url: ${{ secrets.LLM_BASE_URL }} model: ${{ secrets.LLM_MODEL }} + # Reduce if your LLM provider has a lower token limit (default: 32768) + # max_tokens: '16384' + # Increase if your LLM provider is slow (default: 180s) + # timeout: '300' diff --git a/examples/workflow-example.yml b/examples/workflow-example.yml index 90c1256..49d6fd4 100644 --- a/examples/workflow-example.yml +++ b/examples/workflow-example.yml @@ -33,3 +33,6 @@ jobs: # Optional: disable PR comments # post_comment: 'false' + + # Optional: reduce token limit if your LLM provider requires it (default: 32768) + # max_tokens: '16384' diff --git a/review_config.json b/review_config.json index 70a0c3e..06fb495 100644 --- a/review_config.json +++ b/review_config.json @@ -3,10 +3,10 @@ "model": "anthropic/claude-sonnet-4", "base_url": "https://api.opencode.ai/v1", "api_key_env": "LLM_API_KEY", - "timeout": 180, + "timeout": 600, "max_retries": 3, "fallback_model": "anthropic/claude-haiku", - "max_tokens_per_request": 8192, + "max_tokens_per_request": 32768, "token_limit_strategy": "chunk", "chars_per_token": 4 }, diff --git a/review_config_example.json b/review_config_example.json index 0e6893c..ce03b0f 100644 --- a/review_config_example.json +++ b/review_config_example.json @@ -3,10 +3,10 @@ "model": "gpt-4", "base_url": "https://api.openai.com/v1", "api_key_env": "OPENAI_API_KEY", - "timeout": 180, + "timeout": 600, "max_retries": 3, "fallback_model": "gpt-3.5-turbo", - "max_tokens_per_request": 8192 + "max_tokens_per_request": 32768 }, "review": { "critical_rules": [ diff --git a/review_config_rust_example.json b/review_config_rust_example.json index 9b7603c..047413b 100644 --- a/review_config_rust_example.json +++ b/review_config_rust_example.json @@ -3,7 +3,7 @@ "model": "anthropic/claude-sonnet-4", "base_url": "https://api.opencode.ai/v1", "api_key_env": "LLM_API_KEY", - "timeout": 180, + "timeout": 600, "max_retries": 3 }, "prompt": { diff --git a/review_core.py b/review_core.py index dc2d593..1330055 100644 --- a/review_core.py +++ b/review_core.py @@ -53,46 +53,49 @@ def review_outcome(self) -> str: class LLMReviewer: """Handles LLM interactions for code review.""" - DEFAULT_PROMPT = """You are a strict, security-focused code reviewer. Analyze ONLY the added/changed lines in the git diff below. - -Rules: -- Report ONLY issues you can see in the actual diff. Do NOT give generic advice. -- Each issue MUST reference a specific file and line, using format: file.py:42: description -- Added lines in the diff are prefixed with their line number (e.g., "+ 42: code"). Use these exact numbers when referencing issues. -- If no issues found, respond "NONE" for that category. -- Be concise. Do not repeat yourself. -- Before reporting a bug or logic error, carefully trace the COMPLETE control flow — check all branches, shared statements after if/elif/else blocks, and surrounding context. Do NOT report issues based on partial reading of the code. -- The diff includes {context_lines} lines of context around each changed hunk. If a construct (try/except block, multiline function call, class definition) is not fully visible in the provided context, assume it is correctly handled outside the visible area. Do NOT flag incomplete patterns unless you can see the problem directly. -- NEVER reference line numbers or code that is not present in the diff below. Every file:line you cite must correspond to actual content in the diff. -- Do NOT speculate about how code might be called or what data structures might look like outside the diff. Only report issues visible in the provided code. - -CRITICAL ISSUES (block commit): -- Hardcoded credentials, API keys, secrets -- SQL injection, XSS vulnerabilities -- Unsafe functions (eval(), exec(), system()) -- Command injection vulnerabilities -- Buffer overflow risks -{custom_critical_rules} - -WARNINGS (allow commit but flag): -- Actual bugs or logic errors visible in the diff -- Missing error handling for operations that can fail -- Security-relevant input validation gaps -{custom_warnings} - -SUGGESTIONS (only if clearly beneficial): -- Concrete improvements to the changed code -{custom_suggestions} - -{additional_instructions} - -Format: -CRITICAL: file.py:42: issue description -WARNING: file.py:15: issue description -SUGGESTION: file.py:30: suggestion -{code_suggestion_format} -Changes to review: -{diff_content}""" + DEFAULT_SYSTEM_PROMPT = ( + "You are a strict, security-focused code reviewer. Analyze ONLY the added/changed" + " lines in the git diff the user sends you.\n\n" + "Rules:\n" + "- Report ONLY issues you can see in the actual diff. Do NOT give generic advice.\n" + "- Each issue MUST reference a specific file and line, using format: file.py:42: description\n" + '- Added lines in the diff are prefixed with their line number (e.g., "+ 42: code").' + " Use these exact numbers when referencing issues.\n" + '- If no issues found, respond "NONE" for that category.\n' + "- Be concise. Do not repeat yourself.\n" + "- Before reporting a bug or logic error, carefully trace the COMPLETE control flow" + " — check all branches, shared statements after if/elif/else blocks, and surrounding" + " context. Do NOT report issues based on partial reading of the code.\n" + "- The diff includes {{context_lines}} lines of context around each changed hunk." + " If a construct (try/except block, multiline function call, class definition) is not" + " fully visible in the provided context, assume it is correctly handled outside the" + " visible area. Do NOT flag incomplete patterns unless you can see the problem directly.\n" + "- NEVER reference line numbers or code that is not present in the diff." + " Every file:line you cite must correspond to actual content in the diff.\n" + "- Do NOT speculate about how code might be called or what data structures might look" + " like outside the diff. Only report issues visible in the provided code.\n\n" + "CRITICAL ISSUES (block commit):\n" + "- Hardcoded credentials, API keys, secrets\n" + "- SQL injection, XSS vulnerabilities\n" + "- Unsafe shell/code execution functions\n" + "- Command injection vulnerabilities\n" + "- Buffer overflow risks\n" + "{custom_critical_rules}\n\n" + "WARNINGS (allow commit but flag):\n" + "- Actual bugs or logic errors visible in the diff\n" + "- Missing error handling for operations that can fail\n" + "- Security-relevant input validation gaps\n" + "{custom_warnings}\n\n" + "SUGGESTIONS (only if clearly beneficial):\n" + "- Concrete improvements to the changed code\n" + "{custom_suggestions}\n\n" + "{additional_instructions}\n\n" + "Format:\n" + "CRITICAL: file.py:42: issue description\n" + "WARNING: file.py:15: issue description\n" + "SUGGESTION: file.py:30: suggestion\n" + "{code_suggestion_format}" + ) def __init__(self, config, trace: bool = False, trace_llm: bool = False): self.config = config @@ -101,13 +104,16 @@ def __init__(self, config, trace: bool = False, trace_llm: bool = False): self.trace_llm = trace_llm self._setup_logging() - def _build_prompt(self, diff_content: str) -> str: - """Build review prompt from config or use default.""" + def _build_custom_rule_strings(self) -> Tuple[str, str, str, str]: + """Extract and format custom rules from config. + + Returns: + Tuple of (critical_str, warnings_str, suggestions_str, additional) + """ prompt_config = self.config.get("prompt") or {} if not isinstance(prompt_config, dict): prompt_config = {} - # Build placeholder strings first (needed for both custom and default prompts) custom_critical = prompt_config.get("custom_critical_rules") or [] custom_warnings = prompt_config.get("custom_warnings") or [] custom_suggestions = prompt_config.get("custom_suggestions") or [] @@ -122,7 +128,7 @@ def _build_prompt(self, diff_content: str) -> str: if not isinstance(additional, str): additional = "" - if self.config.get("review.check_docstrings", True): + if self.config.get("review.check_docstrings"): docstring_rule = ( "Flag missing docstrings/documentation comments on functions, methods, " "and classes (especially large ones). Suggest language-appropriate format: " @@ -131,45 +137,70 @@ def _build_prompt(self, diff_content: str) -> str: if docstring_rule not in custom_suggestions: custom_suggestions.append(docstring_rule) - critical_str = "\n".join( - f"- {rule}" for rule in custom_critical if isinstance(rule, str) - ) - warnings_str = "\n".join( - f"- {rule}" for rule in custom_warnings if isinstance(rule, str) + return ( + "\n".join(f"- {r}" for r in custom_critical if isinstance(r, str)), + "\n".join(f"- {r}" for r in custom_warnings if isinstance(r, str)), + "\n".join(f"- {r}" for r in custom_suggestions if isinstance(r, str)), + additional, ) - suggestions_str = "\n".join( - f"- {rule}" for rule in custom_suggestions if isinstance(rule, str) + + def _build_system_prompt(self) -> str: + """Build system prompt with reviewer instructions (no diff content).""" + critical_str, warnings_str, suggestions_str, additional = ( + self._build_custom_rule_strings() ) - # Build code suggestion format instructions if self.config.get_code_suggestions_enabled(): - code_suggestion_format = """ -IMPORTANT: When a SUGGESTION references a specific file and line, you MUST include a code block with the exact replacement. Use this format: - -SUGGESTION: file.py:42: description -```suggestion -replacement code here -``` - -For multi-line replacements, specify line range: -SUGGESTION: file.py:42-45: description -```suggestion -replacement code -``` - -Rules for code suggestions: -- Every SUGGESTION with file:line MUST have a ```suggestion block with exact replacement code -- Keep indentation matching the original code -- Only omit the code block for general advice without a specific file:line reference -- Line numbers prefixed on added lines (e.g., "+ 42: code") are metadata only. Do NOT include them in suggested code.""" + code_suggestion_format = ( + "\nIMPORTANT: When a SUGGESTION references a specific file and line, you MUST" + " include a code block with the exact replacement. Use this format:\n\n" + "SUGGESTION: file.py:42: description\n" + "```suggestion\n" + "replacement code here\n" + "```\n\n" + "For multi-line replacements, specify line range:\n" + "SUGGESTION: file.py:42-45: description\n" + "```suggestion\n" + "replacement code\n" + "```\n\n" + "Rules for code suggestions:\n" + "- Every SUGGESTION with file:line MUST have a ```suggestion block with exact replacement code\n" + "- Keep indentation matching the original code\n" + "- Only omit the code block for general advice without a specific file:line reference\n" + '- Line numbers prefixed on added lines (e.g., "+ 42: code") are metadata only.' + " Do NOT include them in suggested code." + ) else: code_suggestion_format = "" - context_lines = self.config.get("output.max_context_lines", 10) + context_lines = self.config.get("output.max_context_lines") + + try: + return self.DEFAULT_SYSTEM_PROMPT.format( + custom_critical_rules=critical_str, + custom_warnings=warnings_str, + custom_suggestions=suggestions_str, + additional_instructions=additional, + code_suggestion_format=code_suggestion_format, + context_lines=context_lines, + ) + except KeyError as e: + self.logger.error( + f"System prompt template error: {e}. Using minimal prompt." + ) + return "You are a strict, security-focused code reviewer." + + def _build_prompt(self, diff_content: str) -> str: + """Build combined prompt for token estimation and custom prompt fallback.""" + prompt_config = self.config.get("prompt") or {} + if not isinstance(prompt_config, dict): + prompt_config = {} - # Check for custom prompt - pass all placeholders custom_prompt = prompt_config.get("custom_prompt") if custom_prompt and isinstance(custom_prompt, str): + critical_str, warnings_str, suggestions_str, additional = ( + self._build_custom_rule_strings() + ) try: return custom_prompt.format( diff_content=diff_content, @@ -177,27 +208,15 @@ def _build_prompt(self, diff_content: str) -> str: custom_warnings=warnings_str, custom_suggestions=suggestions_str, additional_instructions=additional, - code_suggestion_format=code_suggestion_format, - context_lines=context_lines, + code_suggestion_format="", + context_lines=self.config.get("output.max_context_lines"), ) except KeyError as e: self.logger.warning( f"Custom prompt has invalid placeholder: {e}. Using default." ) - try: - return self.DEFAULT_PROMPT.format( - diff_content=diff_content, - custom_critical_rules=critical_str, - custom_warnings=warnings_str, - custom_suggestions=suggestions_str, - additional_instructions=additional, - code_suggestion_format=code_suggestion_format, - context_lines=context_lines, - ) - except KeyError as e: - self.logger.error(f"Prompt template error: {e}. Using minimal prompt.") - return f"Review this code diff for security issues:\n\n{diff_content}" + return self._build_system_prompt() + "\n\nChanges to review:\n" + diff_content def _estimate_tokens(self, text: str) -> int: """Estimate token count from character count.""" @@ -305,7 +324,7 @@ def _handle_token_limit_exceeded( chars_per_token = self.config.get_chars_per_token() max_diff_chars = max_tokens * chars_per_token - prompt_overhead = len(self._build_prompt("")) + prompt_overhead = len(self._build_system_prompt()) available_diff_chars = max_diff_chars - prompt_overhead self.logger.info(f"Token limit exceeded. Using strategy: {strategy}") @@ -509,19 +528,29 @@ def _call_llm(self, diff_content: str) -> ReviewResult: client = self._get_client() model = self.config.get_model() base_url = self.config.get_base_url() + max_response_tokens = self.config.get_max_response_tokens() - prompt = self._build_prompt(diff_content) - prompt_tokens = self._estimate_tokens(prompt) + system_prompt = self._build_system_prompt() + prompt_tokens = self._estimate_tokens(system_prompt + diff_content) self._trace_print(f"LLM Query: model={model}, base_url={base_url}") self._trace_print(f"LLM Query: estimated_tokens={prompt_tokens}") - self._trace_llm_request(prompt, model, base_url) + if self.trace_llm: + self._trace_llm_request( + system_prompt + "\n\nChanges to review:\n" + diff_content, + model, + base_url, + ) try: response = client.chat.completions.create( model=model, - messages=[{"role": "user", "content": prompt}], - temperature=0.1, # Low temperature for consistent analysis + messages=[ + {"role": "system", "content": system_prompt}, + {"role": "user", "content": f"Changes to review:\n{diff_content}"}, + ], + temperature=0.1, # Low temperature for consistent, reproducible analysis + max_tokens=max_response_tokens, ) except OpenAIError as e: self._trace_print(f"LLM Query failed: {e}")