diff --git a/CLAUDE.md b/CLAUDE.md index e0fa2a929..312cb150f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -92,7 +92,7 @@ python3 scripts/aws/filter_log_events_across_streams.py --hours 12 --owner Foxqu - **Auto-write tests**: When creating functions or fixing bugs, always write tests. Bug fix tests must fail without the fix. - **Real captured output only**: BEFORE writing test data, search for existing fixtures (`**/fixtures/*.json`, `**/test_messages.json`) and real data sources (Supabase `llm_requests` table, AWS CloudWatch logs). Use full captured data AS-IS — no stripping, minimizing, or partial extraction. Never hand-craft test dicts when real data exists. If function operates on a subset, test should slice the fixture the same way production code does. -- **Never generate expected output from the function under test**: That's circular. Create expected fixtures independently. +- **Never blindly copy expected values from running the function**: Running the impl to get output is OK, but you MUST manually trace through the logic to verify the result is correct. Understand WHY the output is what it is — don't just paste it. If you can't explain each value, the test is worthless. - **Real cloned repos**: At `../owner/repo`. Run against real repos, never make up file paths. - **ZERO toy tests**: Use full `git ls-files` output as fixtures (hundreds/thousands of files, not 4). Save as fixture files, assert specific real mappings verified manually. Never curate a minimal list. - **Meaningful tests**: Verify actual behavior. No import-only, mock-everything, or string-presence tests. diff --git a/pyproject.toml b/pyproject.toml index 6fefb7fdf..2ed1c9138 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "GitAuto" -version = "1.24.1" +version = "1.24.2" requires-python = ">=3.14" dependencies = [ "annotated-doc==0.0.4", diff --git a/scripts/git/pre_commit_hook.sh b/scripts/git/pre_commit_hook.sh index c36ca8fde..0d4fefb08 100755 --- a/scripts/git/pre_commit_hook.sh +++ b/scripts/git/pre_commit_hook.sh @@ -75,7 +75,7 @@ fi # Print statement check (whole repo, excluding dirs) echo "--- ruff T201 print check ---" -ruff check --select=T201 . --exclude schemas/,.venv/,scripts/ +ruff check --select=T201 . --exclude schemas/,.venv/,scripts/,**/test_*.py,**/conftest.py if [ $? -ne 0 ]; then echo "FAILED: Remove print statements before committing." exit 1 @@ -91,6 +91,21 @@ echo "--- test file check ---" scripts/lint/check_test_files.sh if [ $? -ne 0 ]; then exit 1; fi +# Partial assertion check (assert X in Y is prohibited, use assert X == Y) +echo "--- partial assertion check ---" +scripts/lint/check_partial_assertions.sh +if [ $? -ne 0 ]; then exit 1; fi + +# Private function check (def _xxx is prohibited, inline or own file) +echo "--- private function check ---" +scripts/lint/check_private_functions.sh +if [ $? -ne 0 ]; then exit 1; fi + +# Cast usage check (cast() is prohibited in impl files) +echo "--- cast usage check ---" +scripts/lint/check_cast_usage.sh +if [ $? -ne 0 ]; then exit 1; fi + # Concurrent heavy checks (pylint, pyright, pytest) echo "--- pylint + pyright + pytest (concurrent) ---" scripts/lint/pre_commit_parallel_checks.sh diff --git a/scripts/lint/check_cast_usage.sh b/scripts/lint/check_cast_usage.sh new file mode 100755 index 000000000..d05422deb --- /dev/null +++ b/scripts/lint/check_cast_usage.sh @@ -0,0 +1,35 @@ +#!/bin/bash +# Detect `cast` usage from typing module in staged Python implementation files. +# Rule: No cast in impl files — fix underlying types or use isinstance narrowing. +# Allowed in test files (test_*.py, conftest.py) for TypedDict fixtures. +set -uo pipefail + +STAGED_PY_FILES=$(git diff --cached --name-only --diff-filter=d -- '*.py' \ + | grep -v '^\.\?venv/' \ + | grep -v '^schemas/' \ + | grep -vE '(^|/)test_' \ + | grep -vE '(^|/)conftest\.py$') + +if [ -z "$STAGED_PY_FILES" ]; then + exit 0 +fi + +FAILED=0 +for file in $STAGED_PY_FILES; do + # Match "from typing import ... cast ..." or standalone "cast(" usage + import_match=$(grep -nE '^\s*from\s+typing\s+import\s+.*\bcast\b' "$file" || true) + usage_match=$(grep -nE '\bcast\s*\(' "$file" | grep -vE '^\s*#' || true) + if [ -n "$import_match" ] || [ -n "$usage_match" ]; then + echo "CAST USAGE in $file:" + [ -n "$import_match" ] && echo "$import_match" + [ -n "$usage_match" ] && echo "$usage_match" + FAILED=1 + fi +done + +if [ "$FAILED" -ne 0 ]; then + echo "" + echo "FAILED: cast() is prohibited in implementation files." + echo "Use isinstance narrowing or proper SDK types instead." + exit 1 +fi diff --git a/scripts/lint/check_partial_assertions.sh b/scripts/lint/check_partial_assertions.sh new file mode 100755 index 000000000..553be8f7f --- /dev/null +++ b/scripts/lint/check_partial_assertions.sh @@ -0,0 +1,36 @@ +#!/bin/bash +# Block partial assertions in test files: `assert X in Y`, `assert X not in Y` +# These accept infinite wrong answers. Use `assert result == expected` instead. +set -uo pipefail + +# Get staged test files only +STAGED_TEST_FILES=$(git diff --cached --name-only --diff-filter=d -- '*.py' \ + | grep -E '(^|/)test_' || true) + +if [ -z "$STAGED_TEST_FILES" ]; then + exit 0 +fi + +FAIL=0 + +for file in $STAGED_TEST_FILES; do + # Match `assert X in Y` and `assert X not in Y` (partial assertions) + # Allow: `assert X in {literals}` or `assert X in (literals)` — value validation, not partial + matches=$(grep -nE '^\s+assert\s+.+\s+(not\s+)?in\s+' "$file" \ + | grep -vE '^\s*#' \ + | grep -vE '\bin\s+\{' \ + | grep -vE '\bin\s+\(' || true) + + if [ -n "$matches" ]; then + echo "PARTIAL ASSERTION in $file:" + echo "$matches" + echo " Use 'assert result == expected' instead." + echo "" + FAIL=1 + fi +done + +if [ $FAIL -ne 0 ]; then + echo "FAILED: Replace partial assertions (in/not in) with exact == matches." + exit 1 +fi diff --git a/scripts/lint/check_private_functions.sh b/scripts/lint/check_private_functions.sh new file mode 100755 index 000000000..b14558cb8 --- /dev/null +++ b/scripts/lint/check_private_functions.sh @@ -0,0 +1,35 @@ +#!/bin/bash +# Detect _-prefixed private function definitions in staged Python files. +# Rule: No private functions — inline or create a dedicated file. +# Excludes test files (test_*.py, conftest.py) and __init__.py. +set -uo pipefail + +STAGED_PY_FILES=$(git diff --cached --name-only --diff-filter=d -- '*.py' \ + | grep -v '^\.\?venv/' \ + | grep -v '^schemas/' \ + | grep -vE '(^|/)test_' \ + | grep -vE '(^|/)conftest\.py$' \ + | grep -vE '__init__\.py$') + +if [ -z "$STAGED_PY_FILES" ]; then + exit 0 +fi + +FAILED=0 +for file in $STAGED_PY_FILES; do + # Match "def _name(" but exclude dunder methods "def __name__(" + matches=$(grep -nE '^\s*def\s+_[a-zA-Z]' "$file" \ + | grep -vE 'def\s+__[a-zA-Z_]+__\s*\(' || true) + if [ -n "$matches" ]; then + echo "PRIVATE FUNCTION in $file:" + echo "$matches" + FAILED=1 + fi +done + +if [ "$FAILED" -ne 0 ]; then + echo "" + echo "FAILED: Private functions (_-prefixed) are prohibited." + echo "Either inline the logic or create a dedicated file with a public function." + exit 1 +fi diff --git a/scripts/lint/check_test_files.sh b/scripts/lint/check_test_files.sh index 210b7767d..7a18ea8ea 100755 --- a/scripts/lint/check_test_files.sh +++ b/scripts/lint/check_test_files.sh @@ -38,8 +38,16 @@ if [ -n "$STAGED_IMPL_NEW" ]; then fi # Check 2: Changed impl files with existing test files must have test also staged +# Skip if the staged diff is comment-only (lines starting with # after +/-) if [ -n "$STAGED_IMPL_MODIFIED" ]; then for file in $STAGED_IMPL_MODIFIED; do + # Skip comment-only diffs (all added/removed lines are comments or blank) + if git diff --cached -- "$file" | grep -E '^[+-]' | grep -vE '^(\+\+\+|---)' \ + | grep -vE '^[+-]\s*#' | grep -vE '^[+-]\s*$' | grep -q .; then + : # Has non-comment changes + else + continue + fi dir=$(dirname "$file") base=$(basename "$file") test_file="$dir/test_$base" diff --git a/uv.lock b/uv.lock index 2c8e9dd18..e5953d057 100644 --- a/uv.lock +++ b/uv.lock @@ -596,7 +596,7 @@ wheels = [ [[package]] name = "gitauto" -version = "1.24.1" +version = "1.24.2" source = { virtual = "." } dependencies = [ { name = "annotated-doc" },