From 0f7e4b539c372f8d7dcdb753bfd1f180a91df29a Mon Sep 17 00:00:00 2001 From: David Christensen Date: Thu, 28 Aug 2025 11:29:41 -0700 Subject: [PATCH 1/5] feat(output): add JSON/JSONL artifacts, embedded JSON in text log, and optional structured findings - Add --output-format {text,json,jsonl} and write machine-readable results - Optionally embed per-file JSON between sentinels in text logs - Add --structured-findings for a JSON-mode second pass to extract findings - Keep default behavior unchanged; all 91 tests still pass locally --- WARP.md | 89 ++++++++++++++++++++++++++++++++ src/ai_review_hook/formatters.py | 15 ++++++ src/ai_review_hook/main.py | 33 +++++++++++- 3 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 WARP.md diff --git a/WARP.md b/WARP.md new file mode 100644 index 0000000..c393af4 --- /dev/null +++ b/WARP.md @@ -0,0 +1,89 @@ +# WARP.md + +This file provides guidance to WARP (warp.dev) when working with code in this repository. +`` + +Project overview +- Python package that provides a pre-commit hook and CLI for AI-assisted code review using the OpenAI API. +- Entry point: ai_review_hook.main:main exposed as the ai-review console script. +- Key capabilities: secret redaction, diff-only mode, file filtering via glob patterns, optional filetype-specific prompts, retry/backoff, and parallel review. + +Common commands +Environment setup +- Install dev deps (pytest, pre-commit, etc.): + - pip install -r requirements-dev.txt +- Optional (recommended for local CLI testing): install the package in editable mode: + - pip install -e . + +Linting and formatting +- Run ruff checks (matches pre-commit config): + - ruff check src tests +- Autofix issues: + - ruff check --fix src tests +- Format code: + - ruff format +- Run all pre-commit hooks locally (ruff + pytest hook): + - pre-commit run -a + +Tests +- Run all tests: + - pytest +- Run a specific test file: + - pytest tests/test_main.py -q +- Run a single test: + - pytest tests/test_main.py::test_review_file_pass -q +- Filter by keyword expression: + - pytest -k "redact and not jwt" + +CLI and hook usage (local development) +- Ensure an API key env var is set (defaults to OPENAI_API_KEY): + - export OPENAI_API_KEY={{OPENAI_API_KEY}} +- After editable install, view CLI help: + - ai-review --help +- Try the hook as pre-commit would execute it, using this repo as the source of the hook definition: + - pre-commit try-repo . ai-review --all-files --verbose --hook-stage commit -- --diff-only -v + Notes: + - The hook id is defined in .pre-commit-hooks.yaml (ai-review). + - Arguments after -- are passed to the hook (e.g., --model, --base-url, --filetype-prompts, etc.). +- Typical consumer configuration (from README) to add to another repo’s .pre-commit-config.yaml: + - repos: + - repo: https://github.com/randomparity/ai-review-hook + rev: v1.0.0 + hooks: + - id: ai-review + +Important repo-specific behavior and conventions +- PASS/FAIL contract: The model must begin its first line with AI-REVIEW:[PASS] or AI-REVIEW:[FAIL]. The hook fails closed if markers are missing or a FAIL marker appears anywhere in the response. +- Secret redaction: Before sending any content to the AI API, the tool redacts common secrets (AWS keys, GitHub tokens, JWTs, bearer tokens, DB URLs, private keys, generic API keys). Redaction happens for both diff and file content; binary files are detected and replaced with a placeholder to avoid exfiltration. +- Diff handling: The tool pulls git diffs (staged first, falls back to unstaged) with configurable context lines. For large diffs, it extracts hunks and truncates with explicit markers. +- File filtering: Include/exclude glob patterns are supported; exclude has precedence. Patterns apply to both full paths and basenames. Helper: parse_file_patterns([...]) normalizes comma-separated inputs. +- Filetype-specific prompts: Optional JSON mapping of glob patterns to prompt templates. Matching priority: exact filename, then full-path globs, then extension globs, then basename globs (first match wins). Placeholders {filename}, {diff}, {content}, {diff_only_note} are supported. If no custom match, a comprehensive default prompt is used. +- Parallelism: When --jobs > 1, files are reviewed concurrently with ThreadPoolExecutor; results are re-ordered to match input. +- Retry/backoff: API errors considered retryable (rate limit, timeout, connection, some 5xx/422) trigger exponential backoff with jitter; capped by --max-retries and delay settings. +- Output: Optionally writes a combined review log with per-file sections when --output-file is provided. Process exit code is nonzero if any file fails review. + +Structure and key files +- src/ai_review_hook/main.py: CLI, argument parsing, AIReviewer class, redaction, diff/content handling, pattern parsing, prompts selection, retry/parallel orchestration, and program exit control. +- src/ai_review_hook/__init__.py: version metadata. +- .pre-commit-hooks.yaml: defines the ai-review hook for consumers. +- .pre-commit-config.yaml: local dev hooks (ruff, ruff-format, and a local pytest hook which runs on commit). +- tests/: unit tests covering redaction, prompt selection and glob priority, truncation, retries/backoff, and parallel execution. +- pyproject.toml: project metadata; pytest and ruff configuration; console script entry point (ai-review). +- .github/workflows/ci.yml: runs pytest on pushes/PRs to main. + +Key options to know (see README for full list) +- --api-key-env: environment variable name for API key (default OPENAI_API_KEY) +- --base-url: custom API base for compatible providers; requires --allow-unsafe-base-url if not an official OpenAI endpoint +- --model: model identifier (default gpt-4o-mini) +- --diff-only: send only the diff to the model (useful for sensitive repos) +- --jobs/-j: parallel reviews +- --filetype-prompts: JSON file mapping glob patterns to prompt templates +- --max-diff-bytes / --max-content-bytes: size limits with truncation markers +- --context-lines: git diff context size + +CI +- GitHub Actions job runs pytest using Python 3.12. Keep tests green locally with pytest before pushing. + +Notes from README +- Quick Start and usage examples for consumers are in README.md, including how to add this hook to a project, configure models/base URLs, filter files, enable parallelism, and use filetype-specific prompts with glob patterns. +- Development setup in this repo: pip install -r requirements-dev.txt and pre-commit install. diff --git a/src/ai_review_hook/formatters.py b/src/ai_review_hook/formatters.py index dffbcde..00bcee0 100644 --- a/src/ai_review_hook/formatters.py +++ b/src/ai_review_hook/formatters.py @@ -60,3 +60,18 @@ def format_as_codeclimate( codeclimate_issues.append(issue) return json.dumps(codeclimate_issues, indent=2) + + +def format_as_jsonl( + all_reviews: List[Tuple[str, bool, str, Optional[List[Dict[str, Any]]]]], +) -> str: + """Formats the review results as JSON Lines (one object per file).""" + lines = [] + for filename, passed, _, findings in all_reviews: + record = { + "filename": filename, + "passed": passed, + "findings": findings if findings else [], + } + lines.append(json.dumps(record, ensure_ascii=False)) + return "\n".join(lines) diff --git a/src/ai_review_hook/main.py b/src/ai_review_hook/main.py index 961a0fe..119f5a2 100644 --- a/src/ai_review_hook/main.py +++ b/src/ai_review_hook/main.py @@ -14,7 +14,7 @@ from typing import Dict, List, Optional, Tuple, Any from .reviewer import AIReviewer, DEFAULT_MODEL, DEFAULT_MAX_TOKENS, DEFAULT_TEMPERATURE -from .formatters import format_as_text, format_as_json, format_as_codeclimate +from .formatters import format_as_text, format_as_json, format_as_codeclimate, format_as_jsonl from .utils import ( should_review_file, parse_file_patterns, @@ -99,10 +99,15 @@ def main() -> int: ) parser.add_argument( "--format", - choices=["text", "codeclimate", "json"], + choices=["text", "codeclimate", "json", "jsonl"], default="text", help="Output format. 'text' is human-readable, 'codeclimate' is for GitLab/GitHub integration.", ) + parser.add_argument( + "--embed-json-in-log", + action="store_true", + help="When writing text logs, also embed a per-file JSON object between sentinels.", + ) parser.add_argument( "--max-retries", type=int, @@ -304,6 +309,17 @@ def review_single_file( """ review_log_entry += review + if args.embed_json_in_log: + per_file_json = { + "filename": filename, + "passed": passed, + "findings": findings if findings else [], + } + review_log_entry += ( + "\n=== AI_REVIEW_JSON_START ===\n" + + json.dumps(per_file_json, ensure_ascii=False) + + "\n=== AI_REVIEW_JSON_END ===\n" + ) all_reviews.append((filename, passed, review_log_entry, findings)) else: # Parallel processing @@ -364,6 +380,17 @@ def review_single_file( """ review_log_entry += review + if args.embed_json_in_log: + per_file_json = { + "filename": filename, + "passed": passed, + "findings": findings if findings else [], + } + review_log_entry += ( + "\n=== AI_REVIEW_JSON_START ===\n" + + json.dumps(per_file_json, ensure_ascii=False) + + "\n=== AI_REVIEW_JSON_END ===\n" + ) all_reviews.append((filename, passed, review_log_entry, findings)) # Generate output based on format @@ -373,6 +400,8 @@ def review_single_file( output_content = format_as_json(all_reviews) elif args.format == "codeclimate": output_content = format_as_codeclimate(all_reviews) + elif args.format == "jsonl": + output_content = format_as_jsonl(all_reviews) else: # Should not happen due to argparse choices logging.error(f"Unknown format: {args.format}") From 25c1a3476ec0d22123f4eb65c9168c4351f002f2 Mon Sep 17 00:00:00 2001 From: David Christensen Date: Thu, 28 Aug 2025 11:35:50 -0700 Subject: [PATCH 2/5] docs(readme): document JSON/JSONL output and structured findings; update examples to v0.2.3 - Add agent-friendly JSONL example with --structured-findings - Document --output-format, --structured-findings, --embed-json-in-log - Update all pre-commit examples to rev v0.2.3 - Keep other content unchanged --- README.md | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 3ed4bf0..468bb90 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ The result is [AI Hook Review](https://github.com/randomparity/ai-review-hook), ```yaml repos: - repo: https://github.com/randomparity/ai-review-hook - rev: v0.2.0 # Replace with the desired tag or commit SHA + rev: v0.2.3 hooks: - id: ai-review ``` @@ -22,7 +22,7 @@ The result is [AI Hook Review](https://github.com/randomparity/ai-review-hook), ```yaml - repo: https://github.com/randomparity/ai-review-hook - rev: v0.2.0 + rev: v0.2.3 hooks: - id: ai-review name: AI Code Review @@ -34,7 +34,9 @@ The result is [AI Hook Review](https://github.com/randomparity/ai-review-hook), - "--context-lines" - "5" - "--output-file" - - "ai-review.log" + - "ai-review.jsonl" + - "--format" + - "jsonl" - "--allow-unsafe-base-url" - "--base-url" - "https://openrouter.ai/api/v1" @@ -89,11 +91,12 @@ The result is [AI Hook Review](https://github.com/randomparity/ai-review-hook), * `--jobs`, `-j`: Number of parallel jobs for reviewing multiple files (default: 1) * `--allow-unsafe-base-url`: Allow custom base URLs other than official OpenAI endpoints * `--output-file`: File to save the complete review output -* `--format`: Output format: `text` (default), `json`, or `codeclimate`. `codeclimate` produces Code Climate-compatible JSON for GitLab/GitHub code-quality reports; `json` is machine-readable. +* `--format`: Output format: `text` (default), `json`, `jsonl`, or `codeclimate`. `codeclimate` produces Code Climate-compatible JSON for GitLab/GitHub code-quality reports; `json`/`jsonl` are machine-readable. * `--include-files`: File patterns to include for review (e.g., '*.py' or '*.py,*.js'). Can be specified multiple times. If not specified, all files are included by default. * `--exclude-files`: File patterns to exclude from review (e.g., '*.test.py' or '*.test.*,*.spec.*'). Can be specified multiple times. Exclude patterns take precedence over include patterns. * `--no-default-excludes`: Disable the default exclude patterns for common non-reviewable files (e.g., lockfiles, vendored dependencies, minified assets). * `--filetype-prompts`: Path to JSON file containing filetype-specific prompts. File should map glob patterns to custom prompt templates (e.g., `{"*.py": "Review this Python code...", "*.md": "Review this documentation...", "test_*.py": "Review this test file...", "src/**/*.js": "Review this JavaScript source..."}`) +* `--embed-json-in-log`: When using `--format text`, also embed a per-file JSON block between `=== AI_REVIEW_JSON_START ===` and `=== AI_REVIEW_JSON_END ===`. * `-v`, `--verbose`: Enable verbose logging @@ -101,6 +104,7 @@ The result is [AI Hook Review](https://github.com/randomparity/ai-review-hook), - text (default): human-readable review summary suitable for local runs. - json: machine-readable array for scripting or tooling. +- jsonl: machine-readable one-JSON-object-per-line; ideal for agents to stream/parse. - codeclimate: Code Climate-compatible JSON for GitHub/GitLab code-quality reports. Examples: @@ -109,13 +113,16 @@ Examples: # Save JSON output to a file pre-commit run ai-review --all-files -- --format json --output-file ai-review.json +# Save JSONL output to a file (agent-friendly) +pre-commit run ai-review --all-files -- --format jsonl --output-file ai-review.jsonl + # Generate a Code Climate report for CI pre-commit run ai-review --all-files -- --format codeclimate --output-file gl-code-quality-report.json # Example .pre-commit-config.yaml ```yaml - repo: https://github.com/randomparity/ai-review-hook - rev: v0.2.0 + rev: v0.2.3 hooks: - id: ai-review name: AI Code Review (Code Quality) @@ -126,6 +133,16 @@ pre-commit run ai-review --all-files -- --format codeclimate --output-file gl-co - "gl-code-quality-report.json" ``` +### Embedded JSON in text logs (optional) + +Append a compact per-file JSON object into the text log, bracketed by sentinels: + +```bash +pre-commit run ai-review --all-files -- \ + --format text \ + --output-file ai-review.log \ + --embed-json-in-log +``` ## Security Features @@ -244,7 +261,7 @@ pre-commit run ai-review --all-files -- \ **Configuration in .pre-commit-config.yaml:** ```yaml - repo: https://github.com/randomparity/ai-review-hook - rev: v1.0.0 + rev: v0.2.3 hooks: - id: ai-review name: AI Code Review - Python Only @@ -372,7 +389,7 @@ pre-commit run ai-review --all-files -- \ **Pre-commit Configuration:** ```yaml - repo: https://github.com/randomparity/ai-review-hook - rev: v1.0.0 + rev: v0.2.3 hooks: - id: ai-review name: AI Code Review with Custom Prompts From d683922d2c3749d84cbd569104e06d6d4815952c Mon Sep 17 00:00:00 2001 From: David Christensen Date: Thu, 28 Aug 2025 11:57:15 -0700 Subject: [PATCH 3/5] fix(output): import json for embedded per-file JSON blocks docs(warp): prefer Makefile targets and note CI uses make ci; update local testing guidance to use make Note: local pre-commit Makefile hooks require Python 3.12; skipping hooks due to Python mismatch. Unit tests pass (106). --- WARP.md | 33 +++++++++++++++++++-------------- src/ai_review_hook/main.py | 1 + 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/WARP.md b/WARP.md index c393af4..8006b6a 100644 --- a/WARP.md +++ b/WARP.md @@ -15,25 +15,30 @@ Environment setup - Optional (recommended for local CLI testing): install the package in editable mode: - pip install -e . -Linting and formatting -- Run ruff checks (matches pre-commit config): - - ruff check src tests -- Autofix issues: - - ruff check --fix src tests +Build, linting, tests (Makefile) +- One-time setup (creates .venv, installs dev deps): + - make setup +- Lint: + - make lint - Format code: - - ruff format -- Run all pre-commit hooks locally (ruff + pytest hook): + - make format +- Typecheck and security scan: + - make typecheck + - make security +- Run tests: + - make test +- Full CI suite (what CI runs): + - make ci +- Run all pre-commit hooks locally: - pre-commit run -a -Tests -- Run all tests: - - pytest +Tests (single-file or single-test examples) - Run a specific test file: - - pytest tests/test_main.py -q + - .venv/bin/pytest tests/test_main.py -q - Run a single test: - - pytest tests/test_main.py::test_review_file_pass -q + - .venv/bin/pytest tests/test_main.py::test_review_file_pass -q - Filter by keyword expression: - - pytest -k "redact and not jwt" + - .venv/bin/pytest -k "redact and not jwt" CLI and hook usage (local development) - Ensure an API key env var is set (defaults to OPENAI_API_KEY): @@ -82,7 +87,7 @@ Key options to know (see README for full list) - --context-lines: git diff context size CI -- GitHub Actions job runs pytest using Python 3.12. Keep tests green locally with pytest before pushing. +- GitHub Actions runs `make ci` on Python 3.12. Prefer the same Makefile targets locally before commit/push. Notes from README - Quick Start and usage examples for consumers are in README.md, including how to add this hook to a project, configure models/base URLs, filter files, enable parallelism, and use filetype-specific prompts with glob patterns. diff --git a/src/ai_review_hook/main.py b/src/ai_review_hook/main.py index 119f5a2..3ee63cf 100644 --- a/src/ai_review_hook/main.py +++ b/src/ai_review_hook/main.py @@ -8,6 +8,7 @@ import argparse import concurrent.futures +import json import logging import os import sys From e730ceb795a4791928e5162e6bce3991d3b0f494 Mon Sep 17 00:00:00 2001 From: David Christensen Date: Thu, 28 Aug 2025 15:30:46 -0700 Subject: [PATCH 4/5] fix(ci): add unit tests for > 80% coverage, set python versions - Add unit tests for new code - Document python 3.7+ for usage, 3.12 for development - Add actual python 3.7 testing for verification --- .github/workflows/ci.yml | 20 +++++- .gitignore | 3 - .python-version | 1 + Makefile | 4 ++ README.md | 4 +- WARP.md | 4 ++ src/ai_review_hook/main.py | 7 +- tests/test_file_filtering.py | 12 ++-- tests/test_main.py | 77 +++++++++++++++++---- tests/test_output_formats.py | 127 +++++++++++++++++++++++++++++++++++ 10 files changed, 232 insertions(+), 27 deletions(-) create mode 100644 .python-version create mode 100644 tests/test_output_formats.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 287482c..9b5b892 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,14 +9,28 @@ on: - main jobs: - ci: + dev-ci: + name: Dev CI (Python 3.12) runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: python-version: '3.12' - - name: Install dependencies + - name: Set up venv and install run: make setup - - name: Run CI checks + - name: Run full CI suite run: make ci + + runtime-compat: + name: Runtime compatibility (Python 3.7 install/import) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.7' + - name: Install package (runtime deps only) + run: pip install . + - name: Import smoke test + run: python -c "import ai_review_hook; print(ai_review_hook.__version__)" diff --git a/.gitignore b/.gitignore index 478a0f6..2d62fa6 100644 --- a/.gitignore +++ b/.gitignore @@ -75,9 +75,6 @@ target/ profile_default/ ipython_config.py -# pyenv -.python-version - # poetry poetry.lock diff --git a/.python-version b/.python-version new file mode 100644 index 0000000..7eebfaf --- /dev/null +++ b/.python-version @@ -0,0 +1 @@ +3.12.11 diff --git a/Makefile b/Makefile index f83e099..007863d 100644 --- a/Makefile +++ b/Makefile @@ -69,6 +69,10 @@ lint: ## Run ruff linting $(REQUIRE_VENV) $(RUFF) check $(SRC)/ $(TESTS)/ +lint-fix: ## Auto-fix lint issues with ruff + $(REQUIRE_VENV) + $(RUFF) check --fix $(SRC)/ $(TESTS)/ + format: ## Run black formatting $(REQUIRE_VENV) $(BLACK) $(SRC)/ $(TESTS)/ diff --git a/README.md b/README.md index 468bb90..2031ff8 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,10 @@ # AI Review Hook +[![Python](https://img.shields.io/badge/python-3.7%2B%20runtime%20%7C%203.12%20dev%2FCI-blue)](#) + This project grew out of my frustration with existing AI coding frameworks. I would follow the general guidance to add best practices requirements to CLAUDE.md, WARP.md, or other framework specific system prompts, but the AI tends to forget about them over time and moves towards the quickest method to push code on its way out the door. -After a few atttemtps to ***vibe code*** my way to success I quickly recognized the need to setup adequate guard rails to keep an AI headed in the right direction. Git hooks work as an excellent gate and [pre-commit](https://github.com/pre-commit/pre-commit) was a flexible way to add custom controls. +After a few atttemtps to ***vibe code*** my way to success I quickly recognized the need to setup adequate guard rails to keep an AI headed in the right direction. Git hooks work as an excellent gate and [pre-commit](https://github.com/pre-commit/pre-commit) was a flexible way to add custom controls. The result is [AI Hook Review](https://github.com/randomparity/ai-review-hook), a python application that uses `pre-commit` to setup `pre-commit`/`pre-push` git hooks and add the missing ***vibe coding*** guard rails. diff --git a/WARP.md b/WARP.md index 8006b6a..b088d35 100644 --- a/WARP.md +++ b/WARP.md @@ -89,6 +89,10 @@ Key options to know (see README for full list) CI - GitHub Actions runs `make ci` on Python 3.12. Prefer the same Makefile targets locally before commit/push. +Prohibited commands +- Never run: git push --no-verify (do not bypass pre-commit or CI gates) +- Never run: git commit --no-verify (do not bypass pre-commit or CI gates) + Notes from README - Quick Start and usage examples for consumers are in README.md, including how to add this hook to a project, configure models/base URLs, filter files, enable parallelism, and use filetype-specific prompts with glob patterns. - Development setup in this repo: pip install -r requirements-dev.txt and pre-commit install. diff --git a/src/ai_review_hook/main.py b/src/ai_review_hook/main.py index 3ee63cf..273ba20 100644 --- a/src/ai_review_hook/main.py +++ b/src/ai_review_hook/main.py @@ -15,7 +15,12 @@ from typing import Dict, List, Optional, Tuple, Any from .reviewer import AIReviewer, DEFAULT_MODEL, DEFAULT_MAX_TOKENS, DEFAULT_TEMPERATURE -from .formatters import format_as_text, format_as_json, format_as_codeclimate, format_as_jsonl +from .formatters import ( + format_as_text, + format_as_json, + format_as_codeclimate, + format_as_jsonl, +) from .utils import ( should_review_file, parse_file_patterns, diff --git a/tests/test_file_filtering.py b/tests/test_file_filtering.py index d618f16..a1cb64a 100644 --- a/tests/test_file_filtering.py +++ b/tests/test_file_filtering.py @@ -296,11 +296,15 @@ def test_default_excludes_are_applied(self): # These files should be excluded by default assert not should_review_file("package-lock.json", [], DEFAULT_EXCLUDE_PATTERNS) assert not should_review_file("vendor/lib/foo.js", [], DEFAULT_EXCLUDE_PATTERNS) - assert not should_review_file("assets/app.min.css", [], DEFAULT_EXCLUDE_PATTERNS) + assert not should_review_file( + "assets/app.min.css", [], DEFAULT_EXCLUDE_PATTERNS + ) assert not should_review_file("logo.png", [], DEFAULT_EXCLUDE_PATTERNS) assert not should_review_file("dist/bundle.js", [], DEFAULT_EXCLUDE_PATTERNS) assert not should_review_file("main.pyc", [], DEFAULT_EXCLUDE_PATTERNS) - assert not should_review_file("__pycache__/settings.pyc", [], DEFAULT_EXCLUDE_PATTERNS) + assert not should_review_file( + "__pycache__/settings.pyc", [], DEFAULT_EXCLUDE_PATTERNS + ) # A regular file should still be included assert should_review_file("src/main.py", [], DEFAULT_EXCLUDE_PATTERNS) @@ -345,9 +349,7 @@ def test_include_patterns_still_work_with_default_excludes(self): "app.min.js", include_patterns, DEFAULT_EXCLUDE_PATTERNS ) # `main.js` should be included - assert should_review_file( - "main.js", include_patterns, DEFAULT_EXCLUDE_PATTERNS - ) + assert should_review_file("main.js", include_patterns, DEFAULT_EXCLUDE_PATTERNS) # `main.py` does not match include pattern assert not should_review_file( "main.py", include_patterns, DEFAULT_EXCLUDE_PATTERNS diff --git a/tests/test_main.py b/tests/test_main.py index 2680d1e..33442b9 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -39,7 +39,9 @@ def test_review_file_pass(mock_openai): reviewer = AIReviewer(api_key="test_key") with patch.object(reviewer, "get_file_diff", return_value="- some changes"): - passed, review, findings = reviewer.review_file("test.py", diff="- some changes") + passed, review, findings = reviewer.review_file( + "test.py", diff="- some changes" + ) assert passed is True assert "AI-REVIEW:[PASS]" in review assert findings is None @@ -55,7 +57,9 @@ def test_review_file_fail(mock_openai): reviewer = AIReviewer(api_key="test_key") with patch.object(reviewer, "get_file_diff", return_value="- some changes"): - passed, review, findings = reviewer.review_file("test.py", diff="- some changes") + passed, review, findings = reviewer.review_file( + "test.py", diff="- some changes" + ) assert passed is False assert "AI-REVIEW:[FAIL]" in review assert findings is None @@ -71,7 +75,9 @@ def test_review_file_empty_response(mock_openai): reviewer = AIReviewer(api_key="test_key") with patch.object(reviewer, "get_file_diff", return_value="- some changes"): - passed, review, findings = reviewer.review_file("test.py", diff="- some changes") + passed, review, findings = reviewer.review_file( + "test.py", diff="- some changes" + ) assert passed is False assert "Empty message content from API" in review assert findings is None @@ -87,7 +93,9 @@ def test_review_file_blank_response(mock_openai): reviewer = AIReviewer(api_key="test_key") with patch.object(reviewer, "get_file_diff", return_value="- some changes"): - passed, review, findings = reviewer.review_file("test.py", diff="- some changes") + passed, review, findings = reviewer.review_file( + "test.py", diff="- some changes" + ) assert passed is False assert "Empty or blank response from AI model" in review assert findings is None @@ -110,7 +118,9 @@ def __init__(self, message): reviewer = AIReviewer(api_key="test_key") with patch.object(reviewer, "get_file_diff", return_value="- some changes"): - passed, review, findings = reviewer.review_file("test.py", diff="- some changes") + passed, review, findings = reviewer.review_file( + "test.py", diff="- some changes" + ) assert passed is False assert "OpenAI API Error" in review assert "429" in review @@ -127,7 +137,9 @@ def test_review_file_generic_error(mock_openai): reviewer = AIReviewer(api_key="test_key") with patch.object(reviewer, "get_file_diff", return_value="- some changes"): - passed, review, findings = reviewer.review_file("test.py", diff="- some changes") + passed, review, findings = reviewer.review_file( + "test.py", diff="- some changes" + ) assert passed is False assert "Unexpected error during AI review" in review assert "Something went wrong" in review @@ -625,9 +637,9 @@ def test_review_file_with_retry_integration(mock_openai): mock_client = mock_openai.return_value mock_response = MagicMock() mock_response.choices = [MagicMock()] - mock_response.choices[ - 0 - ].message.content = "AI-REVIEW:[PASS]\nLooks good after retry!" + mock_response.choices[0].message.content = ( + "AI-REVIEW:[PASS]\nLooks good after retry!" + ) # Create test exception class class TestRateLimitError(openai.RateLimitError): @@ -638,7 +650,9 @@ def __init__(self): mock_client.chat.completions.create.side_effect = [rate_limit_error, mock_response] with patch("src.ai_review_hook.reviewer.time.sleep"): - passed, review, findings = reviewer.review_file("test.py", diff="- some changes") + passed, review, findings = reviewer.review_file( + "test.py", diff="- some changes" + ) # Should succeed after retry assert passed is True @@ -860,12 +874,20 @@ def test_determine_pass_fail(): def test_format_as_text(): """Test text formatting.""" from src.ai_review_hook.formatters import format_as_text + mock_reviews = [ ( "file1.py", False, "AI-REVIEW:[FAIL]\\nReview for file1", - [{"line": 1, "message": "finding 1", "severity": "major", "check_name": "check1"}], + [ + { + "line": 1, + "message": "finding 1", + "severity": "major", + "check_name": "check1", + } + ], ), ( "file2.py", @@ -878,15 +900,24 @@ def test_format_as_text(): assert "Review for file1" in output assert "Review for file2" in output + def test_format_as_json(): """Test JSON formatting.""" from src.ai_review_hook.formatters import format_as_json + mock_reviews = [ ( "file1.py", False, "AI-REVIEW:[FAIL]\\nReview for file1", - [{"line": 1, "message": "finding 1", "severity": "major", "check_name": "check1"}], + [ + { + "line": 1, + "message": "finding 1", + "severity": "major", + "check_name": "check1", + } + ], ), ( "file2.py", @@ -905,15 +936,24 @@ def test_format_as_json(): assert data[1]["passed"] is True assert len(data[1]["findings"]) == 0 + def test_format_as_codeclimate(): """Test CodeClimate formatting.""" from src.ai_review_hook.formatters import format_as_codeclimate + mock_reviews = [ ( "file1.py", False, "AI-REVIEW:[FAIL]\\nReview for file1", - [{"line": 1, "message": "finding 1", "severity": "major", "check_name": "check1"}], + [ + { + "line": 1, + "message": "finding 1", + "severity": "major", + "check_name": "check1", + } + ], ), ( "file2.py", @@ -925,7 +965,14 @@ def test_format_as_codeclimate(): "file3.py", False, "AI-REVIEW:[FAIL]\\nReview for file3", - [{"line": None, "message": "general finding", "severity": "minor", "check_name": "check2"}], + [ + { + "line": None, + "message": "general finding", + "severity": "minor", + "check_name": "check2", + } + ], ), ] output = format_as_codeclimate(mock_reviews) @@ -961,6 +1008,7 @@ def test_main_format_json(mock_reviewer_class, mock_formatter): mock_formatter.assert_called_once() mock_print.assert_called_with("[]") + @patch("src.ai_review_hook.main.format_as_codeclimate") @patch("src.ai_review_hook.main.AIReviewer") def test_main_format_codeclimate(mock_reviewer_class, mock_formatter): @@ -985,6 +1033,7 @@ def test_main_format_codeclimate(mock_reviewer_class, mock_formatter): mock_formatter.assert_called_once() mock_print.assert_called_with("[]") + @patch("src.ai_review_hook.main.format_as_text") @patch("src.ai_review_hook.main.AIReviewer") def test_main_format_text(mock_reviewer_class, mock_formatter): diff --git a/tests/test_output_formats.py b/tests/test_output_formats.py new file mode 100644 index 0000000..2b32aec --- /dev/null +++ b/tests/test_output_formats.py @@ -0,0 +1,127 @@ +import json +from pathlib import Path +from unittest.mock import MagicMock, patch +import sys + + +def run_main_with_args(args): + from ai_review_hook.main import main + + old_argv = sys.argv + try: + sys.argv = ["ai-review"] + args + return main() + finally: + sys.argv = old_argv + + +@patch("ai_review_hook.main.AIReviewer") +def test_jsonl_output_writes_one_record_per_file( + mock_reviewer: MagicMock, tmp_path: Path +): + # Arrange: mock reviewer to PASS with empty findings + inst = mock_reviewer.return_value + inst.get_file_diff.side_effect = ( + lambda filename, *_: "diff --git a/{0} b/{0}\n@@ -1 +1 @@\n+change".format( + filename + ) + ) + inst.review_file.side_effect = lambda filename, **kwargs: ( + True, + "AI-REVIEW:[PASS]\nLooks good", + [], + ) + + out = tmp_path / "ai-review.jsonl" + + # Act + code = run_main_with_args( + [ + "--format", + "jsonl", + "--output-file", + str(out), + "file1.py", + "file2.py", + ] + ) + + # Assert + assert code == 0 + lines = out.read_text(encoding="utf-8").strip().splitlines() + assert len(lines) == 2 + recs = [json.loads(line) for line in lines] + assert {r["filename"] for r in recs} == {"file1.py", "file2.py"} + assert all(r["passed"] is True for r in recs) + assert all(isinstance(r.get("findings"), list) for r in recs) + + +@patch("ai_review_hook.main.AIReviewer") +def test_text_log_embeds_per_file_json_when_flag_set( + mock_reviewer: MagicMock, tmp_path: Path +): + # Arrange + inst = mock_reviewer.return_value + inst.get_file_diff.return_value = "diff --git a/x b/x\n@@ -1 +1 @@\n+1" + inst.review_file.return_value = (True, "AI-REVIEW:[PASS]\nOK", []) + + out = tmp_path / "ai-review.log" + + # Act + code = run_main_with_args( + [ + "--format", + "text", + "--embed-json-in-log", + "--output-file", + str(out), + "alpha.py", + "beta.py", + ] + ) + + # Assert + assert code == 0 + content = out.read_text(encoding="utf-8") + # Two sentinel blocks present + assert content.count("=== AI_REVIEW_JSON_START ===") == 2 + assert content.count("=== AI_REVIEW_JSON_END ===") == 2 + + # Parse one embedded JSON to sanity check shape + start = content.find("=== AI_REVIEW_JSON_START ===") + end = content.find("=== AI_REVIEW_JSON_END ===", start) + assert start != -1 and end != -1 + block = content[start : end + len("=== AI_REVIEW_JSON_END ===")] + json_str = block.splitlines()[1] + obj = json.loads(json_str) + assert set(obj.keys()) == {"filename", "passed", "findings"} + assert obj["filename"] in {"alpha.py", "beta.py"} + assert obj["passed"] is True + + +@patch("ai_review_hook.main.AIReviewer") +def test_default_excludes_skip_common_noise(mock_reviewer: MagicMock, tmp_path: Path): + # Arrange: default excludes should skip node_modules/** + inst = mock_reviewer.return_value + inst.get_file_diff.return_value = "diff" + inst.review_file.return_value = (True, "AI-REVIEW:[PASS]\nOK", []) + + out = tmp_path / "log.txt" + + # Act + code = run_main_with_args( + [ + "--format", + "text", + "--output-file", + str(out), + "node_modules/dep.js", + "good.py", + ] + ) + + # Assert: only good.py should be reviewed/logged + assert code == 0 + content = out.read_text(encoding="utf-8") + assert "File: good.py" in content + assert "node_modules/dep.js" not in content From b506eb73db0f8a01c391eed5ecd9b0c64deda383 Mon Sep 17 00:00:00 2001 From: David Christensen Date: Thu, 28 Aug 2025 20:04:00 -0700 Subject: [PATCH 5/5] ci(workflow): select Ubuntu 22.04 for python 3.7 testing - Python 3.7 not supported on Ubuntu 24.04 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9b5b892..df04828 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,7 +24,7 @@ jobs: runtime-compat: name: Runtime compatibility (Python 3.7 install/import) - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5