diff --git a/.claude/hooks/auto_fix_diagnostics.py b/.claude/hooks/auto_fix_diagnostics.py new file mode 100644 index 00000000..3afc6dde --- /dev/null +++ b/.claude/hooks/auto_fix_diagnostics.py @@ -0,0 +1,345 @@ +#!/usr/bin/env python3 +""" +PostToolUse Auto-fix Hook — Detects type errors and surfaces them for fixing. + +Two-hook system: + PostToolUse (this file) → runs pyright on edited file, saves errors to state + PreToolUse (pre_edit_gate.py) → blocks edits to OTHER files until errors fixed + +Key behaviors: +- Runs py_compile (syntax), ruff (lint), pyright (type errors) on edited file +- Runs seedgo checklist for AIPass standards +- Saves type errors to state file for PreToolUse gate +- Surfaces ALL errors in additionalContext so Claude sees them +- Smart batching per-file + +Version: 5.0.0 + +CHANGELOG: + - v5.0.0 (2026-03-17): Replaced mcp__ide__getDiagnostics with direct pyright. + Added state file for PreToolUse gate integration. + Single-file pyright (not whole project). + - v4.3.0 (2026-03-17): Added seedgo checklist integration + - v4.0.0 (2025-11-27): Complete rewrite - actual validation, silent operation +""" + +import json +import sys +import subprocess +from pathlib import Path + +EDIT_TOOLS = ["Edit", "Write", "MultiEdit", "NotebookEdit"] +LAST_FILE_PATH = Path(__file__).parent / ".last_diagnostics_file" +STATE_FILE = Path(__file__).parent / ".diagnostics_state.json" +SKIP_EXTENSIONS = {".md", ".txt", ".log", ".csv", ".html"} + +# AIPass-specific Python patterns to check +PYTHON_PATTERNS = { + "bad_optional": { + "pattern": ": str = None", + "message": "Optional param should use 'str | None = None' pattern" + }, + "logger_debug": { + "pattern": "logger.debug(", + "message": "Use logger.info for SystemLogger (logger.debug not supported)" + }, + "return_error_msg": { + "pattern": "return error_msg", + "message": "Return None for error states, not error_msg string" + }, + "open_no_encoding": { + "pattern": "open(", + "requires_missing": "encoding=", + "message": "open() without encoding='utf-8'" + }, + "log_not_log_operation": { + "pattern": ".log(", + "message": "Use log_operation() with success/error params, not .log()" + }, + "dict_none_no_check": { + "pattern": "Dict | None", + "message": "Dict | None return: Add None check before using (if result is None: return)" + } +} + +# JSON-specific patterns for emoji corruption +JSON_CORRUPTION_CHARS = ['\ufffd', '\x00'] + + +def run_python_checks(file_path: str) -> list[str]: + """Run actual Python validation - returns list of errors.""" + errors = [] + + # 1. Syntax check with py_compile + try: + result = subprocess.run( + [sys.executable, "-m", "py_compile", file_path], + capture_output=True, + text=True, + timeout=5 + ) + if result.returncode != 0: + errors.append(f"SYNTAX: {result.stderr.strip()}") + except Exception: + pass + + # 2. Ruff check (if available) - fast linter + try: + result = subprocess.run( + ["ruff", "check", "--select=E,F,W", "--output-format=text", file_path], + capture_output=True, + text=True, + timeout=10 + ) + if result.stdout.strip(): + for line in result.stdout.strip().split("\n")[:5]: + errors.append(f"LINT: {line}") + except FileNotFoundError: + pass + except Exception: + pass + + # 3. AIPass-specific pattern checks + try: + content = Path(file_path).read_text(encoding="utf-8") + lines = content.split("\n") + + for check in PYTHON_PATTERNS.values(): + pattern = check["pattern"] + message = check["message"] + requires_missing = check.get("requires_missing") + + if requires_missing: + if pattern in content and requires_missing not in content: + errors.append(f"PATTERN: {message}") + continue + + for line in lines: + stripped = line.strip() + if stripped.startswith(("#", '"', "'")): + continue + if f'"{pattern}' in line or f"'{pattern}" in line: + continue + if pattern in line: + errors.append(f"PATTERN: {message}") + break + except Exception: + pass + + return errors + + +def run_pyright_check(file_path: str) -> list[dict]: + """Run pyright on a single file. Returns list of error dicts.""" + # Skip hook files - they don't follow project standards + if '/.claude/hooks/' in file_path: + return [] + + try: + result = subprocess.run( + [sys.executable, "-m", "pyright", "--outputjson", file_path], + capture_output=True, + text=True, + timeout=15 + ) + + try: + data = json.loads(result.stdout) + except (json.JSONDecodeError, ValueError): + return [] + + errors = [] + for diag in data.get("generalDiagnostics", []): + severity = diag.get("severity", "") + if severity == "error": + line = diag.get("range", {}).get("start", {}).get("line", 0) + message = diag.get("message", "Unknown error") + errors.append({ + "line": line, + "message": message[:100] + }) + + return errors[:10] # Max 10 errors + + except FileNotFoundError: + return [] # pyright not installed + except subprocess.TimeoutExpired: + return [] # Timeout — don't block + except Exception: + return [] + + +def save_diagnostics_state(file_path: str, errors: list[dict]): + """Save type errors to state file for PreToolUse gate.""" + try: + if errors: + state = { + "file": str(Path(file_path).resolve()), + "errors": errors + } + STATE_FILE.write_text(json.dumps(state), encoding="utf-8") + else: + # No errors — clear the state + if STATE_FILE.exists(): + STATE_FILE.unlink() + except Exception: + pass + + +def run_json_checks(file_path: str) -> list[str]: + """Run actual JSON validation - returns list of errors.""" + errors = [] + + try: + content = Path(file_path).read_text(encoding="utf-8") + + for char in JSON_CORRUPTION_CHARS: + if char in content: + errors.append(f"EMOJI CORRUPTION: Found corrupted character '{repr(char)}'") + break + + try: + data = json.loads(content) + + if isinstance(data, dict): + for key in ['allowed_emojis', 'emojis', 'emoji_list']: + if key in data and isinstance(data[key], list): + for item in data[key]: + if isinstance(item, str) and len(item) == 1: + if ord(item) < 128 and item not in '\u2713\u2717': + errors.append(f"EMOJI CORRUPTION: Suspicious char '{item}' in {key}") + break + + except json.JSONDecodeError as e: + errors.append(f"JSON SYNTAX: {e.msg} at line {e.lineno}") + + except Exception as e: + errors.append(f"READ ERROR: {e!s}") + + return errors + + +def run_seedgo_checklist(file_path: str) -> list[str]: + """Run seedgo standards checklist — returns violations only.""" + if '/.claude/hooks/' in file_path: + return [] + + try: + result = subprocess.run( + ["drone", "@seedgo", "checklist", file_path], + capture_output=True, + text=True, + timeout=15, + cwd=str(Path.home() / "Projects" / "AIPass") + ) + + if result.returncode != 0: + return [] + + violations = [] + for line in result.stdout.split("\n"): + line = line.strip() + if line.startswith("\u2717"): + violation = line[1:].strip() + if violation: + violations.append(violation) + + return violations[:5] + + except FileNotFoundError: + return [] + except Exception: + return [] + + +def should_skip_file(file_path: str) -> bool: + """Check if file should be skipped.""" + if not file_path: + return True + ext = Path(file_path).suffix.lower() + return ext in SKIP_EXTENSIONS + + +def is_same_file_as_last(file_path: str) -> bool: + """Smart batching DISABLED — always recheck. + + Previously skipped rechecks on the same file, but this caused + errors introduced on second edit to be missed (state file didn't + exist from first clean edit, so skip triggered). The 1.7s pyright + cost per edit is acceptable for correctness. + """ + return False + + +def main(): + """Main hook entry point.""" + try: + input_data = json.load(sys.stdin) + tool_name = input_data.get("tool_name", "") + tool_input = input_data.get("tool_input", {}) + file_path = tool_input.get("file_path", "") + + if tool_name not in EDIT_TOOLS: + return + + if should_skip_file(file_path): + return + + if is_same_file_as_last(file_path): + return + + # Collect all errors + errors = [] + file_type = "" + + if file_path.endswith(".py"): + file_type = "Python" + errors = run_python_checks(file_path) + + # Seedgo standards checklist + seedgo_violations = run_seedgo_checklist(file_path) + for v in seedgo_violations: + errors.append(f"SEEDGO: {v}") + + # Pyright type errors (single file) + type_errors = run_pyright_check(file_path) + for te in type_errors: + errors.append(f"TYPE: L{te['line']}: {te['message']}") + + # Save type errors to state file for PreToolUse gate + save_diagnostics_state(file_path, type_errors) + + elif file_path.endswith(".json"): + file_type = "JSON" + errors = run_json_checks(file_path) + else: + return + + # Build output + if errors: + error_text = "\n".join(f" - {e}" for e in errors) + context = f"""[AUTO-FIX] {len(errors)} error(s) in {Path(file_path).name}: +{error_text} + +Fix these errors in {Path(file_path).name} now. Do not skip or defer.""" + + output = { + "hookSpecificOutput": { + "hookEventName": "PostToolUse", + "additionalContext": context + }, + "systemMessage": f"[AUTO-FIX] {len(errors)} error(s) — fix before continuing" + } + print(json.dumps(output)) + else: + output = { + "systemMessage": "[diagnostics] ok" + } + print(json.dumps(output)) + + except Exception: + pass # Silent fail + + +if __name__ == "__main__": + main() diff --git a/.claude/hooks/notification_sound.py b/.claude/hooks/notification_sound.py new file mode 100644 index 00000000..3492e707 --- /dev/null +++ b/.claude/hooks/notification_sound.py @@ -0,0 +1,37 @@ +#!/usr/bin/env python3 +"""Notification Hook — Plays sound when AI needs permission.""" + +import json +import sys +import subprocess +from pathlib import Path + +SOUNDS_DIR = Path(__file__).parent.parent / "sounds" +SOUND_FILE = SOUNDS_DIR / "mixkit-clear-announce-tones-2861.wav" + + +def play_sound() -> None: + if not SOUND_FILE.exists(): + return + try: + subprocess.Popen( + ["aplay", "-q", str(SOUND_FILE)], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + except Exception: + pass + + +def main(): + try: + hook_data = json.loads(sys.stdin.read()) + if hook_data.get("hook_event_name") == "Notification": + play_sound() + except Exception: + pass + sys.exit(0) + + +if __name__ == "__main__": + main() diff --git a/.claude/hooks/stop_sound.py b/.claude/hooks/stop_sound.py new file mode 100644 index 00000000..59236ff6 --- /dev/null +++ b/.claude/hooks/stop_sound.py @@ -0,0 +1,38 @@ +#!/usr/bin/env python3 +"""Stop Hook — Plays achievement bell when AI finishes responding.""" + +import json +import sys +import subprocess +from pathlib import Path + +SOUNDS_DIR = Path(__file__).parent.parent / "sounds" +SOUND_FILE = SOUNDS_DIR / "mixkit-achievement-bell-600.wav" + + +def play_sound() -> None: + if not SOUND_FILE.exists(): + return + try: + subprocess.Popen( + ["aplay", "-q", str(SOUND_FILE)], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + except Exception: + pass + + +def main(): + try: + hook_data = json.loads(sys.stdin.read()) + if hook_data.get("hook_event_name") == "Stop": + if not hook_data.get("stop_hook_active", False): + play_sound() + except Exception: + pass + sys.exit(0) + + +if __name__ == "__main__": + main() diff --git a/.claude/hooks/tool_use_sound.py b/.claude/hooks/tool_use_sound.py new file mode 100644 index 00000000..fe09c5b3 --- /dev/null +++ b/.claude/hooks/tool_use_sound.py @@ -0,0 +1,40 @@ +#!/usr/bin/env python3 +"""Tool Use Hook — Plays key press sound when AI uses tools.""" + +import json +import sys +import subprocess +from pathlib import Path + +SOUNDS_DIR = Path(__file__).parent.parent / "sounds" +SOUND_FILE = SOUNDS_DIR / "mixkit-atm-cash-machine-key-press-2841.wav" + +SOUND_TOOLS = ["Bash", "Edit", "MultiEdit", "Write", "Read", "Grep", "Glob"] + + +def play_sound() -> None: + if not SOUND_FILE.exists(): + return + try: + subprocess.Popen( + ["aplay", "-q", str(SOUND_FILE)], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + except Exception: + pass + + +def main(): + try: + hook_data = json.loads(sys.stdin.read()) + if hook_data.get("hook_event_name") == "PreToolUse": + if hook_data.get("tool_name", "") in SOUND_TOOLS: + play_sound() + except Exception: + pass + sys.exit(0) + + +if __name__ == "__main__": + main() diff --git a/Dockerfile b/Dockerfile index ee6297cc..4d574030 100644 --- a/Dockerfile +++ b/Dockerfile @@ -27,4 +27,4 @@ USER 1000 ENV PATH="/opt/venv/bin:$PATH" EXPOSE 8080 -ENTRYPOINT ["/usr/bin/entrypoint.sh", "--bind-addr", "0.0.0.0:8080", "--auth", "none", "/home/coder/workspace"] +ENTRYPOINT ["/usr/bin/entrypoint.sh", "--bind-addr", "0.0.0.0:8080", "--auth", "password", "/home/coder/workspace"] diff --git a/src/aipass/seedgo/.seedgo/bypass.json b/src/aipass/seedgo/.seedgo/bypass.json index cd67e3e5..3486cde6 100644 --- a/src/aipass/seedgo/.seedgo/bypass.json +++ b/src/aipass/seedgo/.seedgo/bypass.json @@ -81,14 +81,9 @@ "standard": "debug_print", "reason": "print() appears in string patterns/regex, not as executable code" }, - { - "file": "test_coverage_check.py", - "standard": "testing", - "reason": "Checker file for the test_coverage standard — not a test file despite 'test' in name" - }, { "file": "test_quality_check.py", - "standard": "testing", + "standard": "error_handling", "reason": "Checker file for the test_quality standard — not a test file despite 'test' in name" }, { diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling.md b/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling.md index 96713f06..2ba72fc5 100644 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling.md +++ b/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling.md @@ -1,577 +1,113 @@ # Error Handling Standards -**Status:** Active - 3-Tier Logging Architecture -**Date:** 2025-11-21 -**Last Major Update:** 2026-01-31 - Added ERROR vs WARNING log level guidelines - -## What This Covers - -Error handling, logging architecture, exception patterns, and the 3-tier separation of concerns for logging and error management across all AIPass branches. +**Status:** v1.0 — Consolidated from testing standard +**Date:** 2026-03-27 --- -## The 3-Tier Architecture +## What It Is -**Core Principle:** Logging responsibility follows the architectural hierarchy. +The error handling standard validates that try/except blocks handle errors meaningfully. Silent failures (`except: pass`) hide bugs and erode trust in the codebase. -``` -Entry Point (flow.py, seedgo.py) - ↓ (operational logging via Prax) -Module (apps/modules/*.py) - ↓ (business logging via Prax - catches and logs everything) -Handler (apps/handlers/**/*.py) - ↓ (operational logging via Prax - returns results to modules) -``` - -**Why This Matters:** -- **Scale:** 17 branches × 50+ handlers = 850+ files to manage -- **Auditability:** Check one directory (modules/) for all business logging -- **Testability:** Automated scans verify compliance -- **Consistency:** Single pattern across entire ecosystem +Previously named "testing" — renamed for clarity. This checker has nothing to do with tests. It checks error handling patterns in production code. --- -## Tier 1: Entry Points +## What the Checker Scans For -**Files:** `flow.py`, `seedgo.py`, `prax.py`, `drone.py`, `ai_mail.py` +This is a **file-level** checker (`AUDIT_SCOPE = "all_files"`) that runs on every Python file. -**Prax Import:** YES (minimal) +### Detection Logic -**Logging Scope:** Operational only +1. Counts `try:` blocks in the file +2. If no try/except blocks exist → not applicable (100%) +3. Scans for silent failure patterns: + - `except: pass` + - `except Exception: pass` + - Any except block where the only statement is `pass` +4. Tracks docstrings to avoid false positives in string content -**What to Log:** -```python -from aipass.prax.apps.modules.logger import system_logger as logger +### What Counts as Silent -# ✓ Module discovery results -logger.info(f"Discovered {len(modules)} modules") - -# ✓ Command routing -logger.info(f"Routing command '{command}' to module") - -# ✓ Help system access -logger.info("Displaying help information") - -# ✓ Connection tests -logger.info("Testing Prax connection") -``` - -**What NOT to Log:** -```python -# ✗ Business errors (module logs these) -logger.error("Failed to create plan") # NO! - -# ✗ Handler exceptions (module logs these) -logger.error("Handler threw exception") # NO! - -# ✗ Validation failures (module logs these) -logger.warning("Invalid input") # NO! -``` - -**Characteristics:** -- Operates standalone (help, list modules) -- Same template for all branches (rename only) -- Minimal logging - operational visibility only -- Does NOT catch or log business errors +An except block is "silent" if: +- The only non-comment statement after `except ...:` is `pass` +- No logging, no return, no re-raise, no other statements --- -## Tier 2: Modules (THE LOGGING LAYER) +## Code Examples -**Location:** `apps/modules/*.py` +### Violation -**Prax Import:** YES (required) - -**Logging Scope:** ALL business logging - -**Pattern:** ```python -from aipass.prax.apps.modules.logger import system_logger as logger - -MODULE_NAME = "create_plan" - -def handle_command(command: str, args: List[str]) -> bool: - """Handle command with full error logging""" - - try: - # Call handler - result = create_plan_handler(location, subject) - - if result['success']: - logger.info(f"[{MODULE_NAME}] Plan created successfully") - return True - else: - logger.error(f"[{MODULE_NAME}] Failed to create plan: {result['error']}") - return False - - except ValueError as e: - logger.error(f"[{MODULE_NAME}] Validation error: {e}") - return False - except FileNotFoundError as e: - logger.error(f"[{MODULE_NAME}] File not found: {e}") - return False - except Exception as e: - logger.error(f"[{MODULE_NAME}] Unexpected error: {e}") - return False +try: + result = api_call() +except: + pass # Silent failure — error swallowed ``` -**What to Log:** +### Fix + ```python -# ✓ All errors -logger.error(f"[{MODULE_NAME}] Operation failed: {error_msg}") - -# ✓ All warnings -logger.warning(f"[{MODULE_NAME}] Deprecated feature used") - -# ✓ Important info -logger.info(f"[{MODULE_NAME}] Processing 15 items") - -# ✓ Workflow boundaries -logger.info(f"[{MODULE_NAME}] Starting workflow") -logger.info(f"[{MODULE_NAME}] Workflow complete") - -# ✓ Handler exceptions (caught and logged) -except ValueError as e: - logger.error(f"[{MODULE_NAME}] Handler validation failed: {e}") +try: + result = api_call() +except Exception as e: + logger.error(f"API call failed: {e}") + return {"success": False, "error": str(e)} ``` --- -## Log Level Guidelines: ERROR vs WARNING +## Error Handling Philosophy -**Core Distinction:** ERROR triggers Prax escalation, WARNING does not. +**Fix error handling BEFORE fixing bugs.** -| Level | Use For | Examples | -|-------|---------|----------| -| `logger.error()` | **System failures** - things that shouldn't happen | File I/O errors, crashes, dependency failures, unexpected exceptions, corruption | -| `logger.warning()` | **User input issues** - expected validation failures | "Plan not found", "Field required", "Invalid format", "Already exists" | - -**Why This Matters:** -- Prax log_watcher escalates ERROR-level entries to AI_MAIL -- User typos shouldn't trigger system alerts -- ERROR = something is broken; WARNING = user needs to try again - -**Pattern:** -```python -# ✓ System failure → ERROR (Prax escalates) -logger.error(f"[{MODULE_NAME}] Failed to write file: {e}") -logger.error(f"[{MODULE_NAME}] Database connection lost") -logger.error(f"[{MODULE_NAME}] Unexpected exception: {e}") - -# ✓ User input issue → WARNING (no escalation) -logger.warning(f"[{MODULE_NAME}] Plan {plan_id} not found") -logger.warning(f"[{MODULE_NAME}] Required field 'subject' missing") -logger.warning(f"[{MODULE_NAME}] Invalid format: expected PLAN0001") -``` - -**CLI Display vs System Logging:** -- CLI can still show "ERROR" text to user (feedback) -- System logs use WARNING level (no Prax escalation) -- These are independent concerns - -**Decision:** Approved 2026-01-31 - Rollout to all branches -``` - -**Logging Pattern Rules:** -1. **Always prefix with MODULE_NAME:** `[{MODULE_NAME}]` -2. **Catch all exceptions from handlers** -3. **Log with context** - what failed, why it matters -4. **Return bool to entry point** - True for success, False for failure - -**Responsibilities:** -- Orchestrate handler calls -- Catch exceptions from handlers -- Log everything that goes wrong -- Provide context for debugging -- Return success/failure to entry point +If errors lie, you can't debug effectively. The debug cycle: +1. Fix error handling — make errors tell the truth +2. Then fix the actual bug +3. See clean pass with honest outputs --- -## Tier 3: Handlers (THE WORKER LAYER) +## Scoring -**Location:** `apps/handlers/**/*.py` - -**Prax Import:** YES (allowed and encouraged) - -**Logging Scope:** Operational logging via Prax system_logger - -**Updated 2026-02-27:** Handlers MAY (and should) use Prax `system_logger` for logging. The previous prohibition has been lifted. ONE logging system everywhere — Prax. - -### Handler Logging Patterns - -**Pattern A: Regular Handlers (called by modules)** - -Regular handlers called by module orchestrators return results. The module logs on their behalf. These handlers MAY also log directly via Prax if they need to record operational details. - -```python -from aipass.prax.apps.modules.logger import system_logger as logger - -def create_plan_handler(location: str, subject: str) -> dict: - """Create a plan file""" - if not location: - return { - 'success': False, - 'data': None, - 'error': 'Location is required' - } - - try: - plan_path = Path(location) / f"PLAN{number:04d}.md" - plan_path.write_text(content) - logger.info(f"Plan created at {plan_path}") - return {'success': True, 'data': {'path': str(plan_path)}, 'error': None} - except PermissionError as e: - return {'success': False, 'data': None, 'error': f'Permission denied: {e}'} -``` - -**Pattern B: Autonomous Handlers (plugins, services, cron)** - -Plugin and service handlers are mini entry points — invoked by schedulers, daemons, or running as long-lived processes. They MUST use Prax system_logger. - -```python -from aipass.prax.apps.modules.logger import system_logger as logger - -# Plugin triggered by cron — no calling module exists -def run(): - logger.info("Daily audit starting") - results = run_audit() - logger.info(f"Audit complete: {results['score']}%") -``` - -**What Handlers MUST NOT Do:** -```python -# ✗ Use stdlib logging instead of Prax -import logging -log = logging.getLogger(__name__) # NO! Use Prax system_logger - -# ✗ Create local FileHandler (bypasses system_logs) -handler = logging.FileHandler('logs/output.log') # NO! Prax handles routing -``` - -**Characteristics:** -- Workers — domain-specific tasks -- Log via Prax system_logger (same system as modules) -- Return results to calling modules when applicable -- Testable in isolation +- **Scope:** `AUDIT_SCOPE = "all_files"` — runs on every Python file via `check_module()` +- **Score formula:** `passed_checks / total_checks * 100` +- **No try/except blocks:** Score = 100 (not applicable) +- **Overall pass threshold:** 75% --- -## Validation Rules +## Bypass -**Automated Compliance Checks:** +Bypass rules are configured in `.seedgo/bypass.json`. Supports: -```bash -# All modules MUST import Prax -grep -r "from aipass.prax.apps.modules.logger import" apps/modules/*.py +- **Standard-level bypass:** Skip the entire `error_handling` standard for a file +- **File-level bypass:** Match by file path substring +- **Line-level bypass:** Skip specific lines -# Handlers SHOULD import Prax (no longer prohibited) -grep -r "from aipass.prax.apps.modules.logger import" apps/handlers/**/*.py - -# Handlers MUST NOT use stdlib logging.getLogger -grep -r "logging.getLogger" apps/handlers/**/*.py # Should find NOTHING - -# All modules MUST have error logging -grep -r "logger.error" apps/modules/*.py # Should find ALL modules -``` - -**Manual Validation:** -1. Run entry point without args - should list modules -2. Run module without args - should list handlers -3. Trigger errors - verify Prax logs capture them -4. Check `system_logs/` directory for expected output - ---- - -## Migration Checklist - -**For Existing Branches (migrating handlers to Prax logging):** - -1. **Scan handlers for stdlib logging** - ```bash - grep -r "logging.getLogger" apps/handlers/ - ``` - -2. **Replace stdlib with Prax system_logger** - - Replace `import logging` / `logging.getLogger()` with `from aipass.prax.apps.modules.logger import system_logger as logger` - - Remove any `logging.FileHandler()` creation (Prax handles routing) - - Keep logger calls as-is (logger.info, logger.error, etc.) - -3. **Verify modules still import Prax** - - Import Prax in all modules - - Add try/except around handler calls - - Log all errors with MODULE_NAME prefix - -4. **Test compliance** - - Run `drone @seedgo audit @branch` - - Verify logs appear in system_logs/ - - Verify no stdlib logging.getLogger remains - ---- - -## Common Patterns - -### Module Error Handling Template - -```python -from aipass.prax.apps.modules.logger import system_logger as logger - -MODULE_NAME = "module_name" - -def handle_command(command: str, args: List[str]) -> bool: - """Standard error handling pattern""" - - try: - # Parse arguments - parsed_args = parse_arguments(args) - - # Call handler - result = handler_function(**parsed_args) - - # Handle result - if isinstance(result, dict) and 'success' in result: - if result['success']: - logger.info(f"[{MODULE_NAME}] Operation successful") - return True - else: - # WARNING for user input issues, ERROR for system failures - logger.warning(f"[{MODULE_NAME}] {result['error']}") - return False - else: - logger.info(f"[{MODULE_NAME}] Operation complete") - return True - - # User input issues → WARNING (no Prax escalation) - except ValueError as e: - logger.warning(f"[{MODULE_NAME}] Validation error: {e}") - return False - except FileNotFoundError as e: - # WARNING if user-provided path, ERROR if system config - logger.warning(f"[{MODULE_NAME}] File not found: {e}") - return False - - # System failures → ERROR (Prax escalates) - except PermissionError as e: - logger.error(f"[{MODULE_NAME}] Permission denied: {e}") - return False - except Exception as e: - logger.error(f"[{MODULE_NAME}] Unexpected error: {e}") - return False -``` - -### Handler Return Template - -```python -def handler_function(param1: str, param2: int) -> dict: - """ - Handler that returns status dict - - Args: - param1: Description - param2: Description - - Returns: - dict: {'success': bool, 'data': Any, 'error': str} - """ - # Validate - if not param1: - return { - 'success': False, - 'data': None, - 'error': 'param1 is required' - } - - # Do work - try: - result = do_work(param1, param2) - return { - 'success': True, - 'data': result, - 'error': None - } - except SomeError as e: - return { - 'success': False, - 'data': None, - 'error': str(e) - } +Example bypass rule: +```json +{ + "standard": "error_handling", + "file": "legacy_module.py", + "reason": "Legacy code — silent catches are intentional during migration" +} ``` --- -## Why This Pattern Works +## History -**One System Everywhere:** -- Prax system_logger in entry points, modules, AND handlers -- Same dual output (local + system_logs), same rotation, same monitor feed -- No more two-logging-system inconsistency - -**Auditability:** -- Automated Seedgo checks verify Prax imports across ALL tiers -- `drone @seedgo audit @branch` catches stdlib logging violations -- One pattern to check: does it use Prax? - -**Testability:** -- Automated scans verify compliance -- Pytests validate checker behavior -- New branches follow template - -**Consistency:** -- All branches, all tiers use same pattern -- Training new AI instances is simple -- No confusion about "where should I log this?" — always Prax +- Renamed from `testing` to `error_handling` (2026-03-27) +- The `check_test_functions()` feature was removed (redundant with test_quality standard) +- Original checker focused on two things: error handling + test function presence +- Now focused solely on error handling, which is what it actually checks --- ## Reference -**See Flow for working example:** -- `/flow/apps/modules/` -- `/flow/apps/handlers/` - -**See Seedgo for reference implementation:** -- `/seedgo/apps/modules/` -- `/seedgo/apps/handlers/` - -**Related Standards:** -- Architecture (3-tier pattern) -- Modules (orchestration responsibility) -- Handlers (pure workers, independence) - ---- - -## Documented Exceptions - -### Prax Logging Infrastructure - -Prax handlers in `apps/handlers/logging/` use Python's stdlib `logging` instead of `system_logger`. This is **intentional** to avoid circular dependencies — the logging handlers ARE the logging infrastructure. - -**Status:** Documented in `/prax/.seedgo/bypass.json` - -### Trigger Infrastructure Handlers - -Trigger's `log_watcher.py` and `error_registry.py` use stdlib logging to avoid recursion — Prax logging triggers the event pipeline that trigger watches. These handlers will migrate to Prax `direct_log()` when available. - -**Status:** Documented in bypass rules, pending Prax direct_log() support - ---- - -## Service Logging Standards - -**Added:** 2026-02-04 -**Applies To:** Services that handle user-facing interactions (bridges, APIs, bots) - -### The Gap This Addresses - -Services like Telegram bridges, API endpoints, and chat interfaces need more than operational logging. They require: -1. **Content logging** - meaningful records of what happened -2. **Audit trails** - user-facing interactions preserved -3. **Clear location conventions** - where different log types go - -### What Services Should Log - -**Operational Logs** (Prax-based, existing pattern): -```python -# Status, errors, lifecycle - goes to ~/system_logs/ -logger.info(f"[{SERVICE_NAME}] Bridge started on port 8080") -logger.error(f"[{SERVICE_NAME}] Connection lost: {e}") -``` - -**Content Logs** (Service-specific, new requirement): -```python -# Meaningful interaction records - goes to service's logs/ directory -# Example: Chat bridge logging -content_log = { - "timestamp": "2026-02-04T10:30:00Z", - "direction": "inbound", # or "outbound" - "user": "user_id", - "content": "User message text", - "response": "Bot response text", - "metadata": {"channel": "telegram", "chat_id": "12345"} -} -``` - -### Log Location Conventions - -| Log Type | Location | Purpose | -|----------|----------|---------| -| **System/operational** | `~/system_logs/` | Prax-captured, cross-branch aggregation | -| **Service content** | `/logs/` | Service-specific content logs | -| **Audit trails** | `/logs/audit/` | User-facing interaction records | - -**Examples:** -``` -~/system_logs/api_telegram.log # Prax operational log -~/aipass_core/api/logs/chat_history.log # Chat content log -~/aipass_core/api/logs/audit/ # Interaction audit trail -``` - -### Requirements for User-Facing Services - -Services that interact with users (bots, bridges, chat interfaces) MUST: - -1. **Log meaningful content, not just status** - - ✓ "Received message from user X: 'hello'" + "Sent response: 'Hi!'" - - ✗ "Message received" + "Response sent" - -2. **Preserve audit trails for interactions** - - Chat messages and responses - - API requests and responses - - Command invocations and results - -3. **Use appropriate log locations** - - Operational → `~/system_logs/` (via Prax) - - Content → `/logs/` (service-managed) - -4. **Include sufficient context** - - Timestamp - - User/source identifier - - Direction (inbound/outbound) - - Content (message, command, response) - - Relevant metadata - -### Content Log Pattern - -```python -import json -from datetime import datetime -from pathlib import Path - -class ContentLogger: - def __init__(self, service_name: str, branch_path: Path): - self.log_dir = branch_path / "logs" - self.log_dir.mkdir(exist_ok=True) - self.log_file = self.log_dir / f"{service_name}_content.log" - - def log_interaction(self, direction: str, user: str, content: str, - response: str = None, metadata: dict = None): - entry = { - "timestamp": datetime.utcnow().isoformat() + "Z", - "direction": direction, - "user": user, - "content": content, - "response": response, - "metadata": metadata or {} - } - with open(self.log_file, "a") as f: - f.write(json.dumps(entry) + "\n") -``` - -### Why This Matters - -**Debugging:** When a user reports "the bot said something wrong", content logs show exactly what happened. - -**Auditing:** Compliance, troubleshooting, and understanding system behavior require interaction records. - -**Learning:** Patterns in user interactions inform future development. - -**Accountability:** Knowing what the system actually said/did, not just that it "worked" or "failed". - ---- - -## Decision Record - -**Original Decision:** 2025-11-21 — 3-tier logging, handlers prohibited from Prax -**Updated:** 2026-02-27 — Unified Prax logging everywhere (FPLAN-0382) -**Decision Maker:** User -**Applies To:** All AIPass branches -**Status:** Active — handlers now use Prax system_logger -**RFC:** Commons #165 — Handler & Plugin Logging +- **Checker:** `error_handling_check.py` +- **Scope:** `all_files` +- **Entry point:** `check_module(module_path, bypass_rules)` +- **Standard label:** `ERROR_HANDLING` diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling_check.py b/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling_check.py index 9025ea54..e65e85c4 100644 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling_check.py +++ b/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling_check.py @@ -9,16 +9,12 @@ """ Error Handling Standards Checker Handler -Validates module compliance with AIPass 3-tier logging standards. -- Modules: MUST import Prax, logger.error() for system failures only -- Handlers: MAY import Prax for info/warning, MUST NOT use logger.error() -- stdlib logging.getLogger() prohibited everywhere +Validates error handling compliance — detects silent failures +(bare except: pass) in production code. """ -import re from pathlib import Path -from typing import Dict, List - +from typing import Dict, List, Optional from aipass.prax import logger from aipass.seedgo.apps.handlers.json import json_handler @@ -30,14 +26,11 @@ def is_bypassed(file_path: str, standard: str, line: int | None = None, bypass_r if not bypass_rules: return False for rule in bypass_rules: - # Must match standard if rule.get('standard') and rule.get('standard') != standard: continue - # Must match file (check if rule file path is in the full path) rule_file = rule.get('file', '') if rule_file and rule_file not in file_path: continue - # Check line-specific bypass rule_lines = rule.get('lines', []) if rule_lines and line is not None and line not in rule_lines: continue @@ -46,31 +39,10 @@ def is_bypassed(file_path: str, standard: str, line: int | None = None, bypass_r def check_module(module_path: str, bypass_rules: list | None = None) -> Dict: - """ - Check if module follows 3-tier error handling standards - - Args: - module_path: Path to Python module to check - bypass_rules: Optional list of bypass rules to skip certain checks - - Returns: - dict: { - 'passed': bool, # Overall pass/fail - 'checks': [ # Individual check results - { - 'name': str, # Check name - 'passed': bool, # Pass/fail - 'message': str, # Details - } - ], - 'score': int, # 0-100 percentage - 'standard': str # Standard name - } - """ + """Check if module follows error handling standards""" checks = [] path = Path(module_path) - # Check if entire standard is bypassed for this file if is_bypassed(module_path, 'error_handling', bypass_rules=bypass_rules): return { 'passed': True, @@ -79,7 +51,6 @@ def check_module(module_path: str, bypass_rules: list | None = None) -> Dict: 'standard': 'ERROR_HANDLING' } - # Validate file exists if not path.exists(): return { 'passed': False, @@ -88,7 +59,6 @@ def check_module(module_path: str, bypass_rules: list | None = None) -> Dict: 'standard': 'ERROR_HANDLING' } - # Read file try: with open(path, 'r', encoding='utf-8') as f: content = f.read() @@ -102,51 +72,16 @@ def check_module(module_path: str, bypass_rules: list | None = None) -> Dict: 'standard': 'ERROR_HANDLING' } - # Determine file type - is_handler = '/handlers/' in module_path - is_module = '/modules/' in module_path - is_entry_point = path.name.endswith('.py') and '/apps/' in module_path and path.parent.name == 'apps' + # Only check: Error handling (for all files, not just non-test files) + error_handling_check = check_error_handling(content, lines, module_path) + if error_handling_check: + checks.append(error_handling_check) - # Check 1: Modules MUST import Prax - # Skip prax's own modules (can't import itself) and cli modules (prax depends on cli — circular) - is_prax_module = '/prax/' in module_path - is_cli_module = '/cli/' in module_path - if is_module and not is_prax_module and not is_cli_module: - prax_import_check = check_module_has_prax(content, module_path, bypass_rules) - checks.append(prax_import_check) - - # Check 2: Handlers MUST NOT use stdlib logging.getLogger - if is_handler: - no_stdlib_check = check_handler_no_stdlib_logging(content, module_path, bypass_rules) - checks.append(no_stdlib_check) - - # Note: Handlers MAY import Prax and use logger.info/warning/error (DPLAN-0040) - # Only stdlib logging.getLogger() is prohibited (check 2 above) - - # Check 4: Modules should have error logging - if is_module: - error_logging_check = check_module_error_logging(content) - checks.append(error_logging_check) - - # Check 5: Modules - ERROR vs WARNING usage - if is_module: - error_warning_check = check_error_vs_warning_usage(lines, module_path, bypass_rules) - checks.append(error_warning_check) - - # If not module or handler, skip checks - if not is_module and not is_handler and not is_entry_point: - return { - 'passed': True, - 'checks': [{'name': 'Error handling check', 'passed': True, 'message': 'Not a module or handler (skipped)'}], - 'score': 100, - 'standard': 'ERROR_HANDLING' - } - - # Calculate score + # If no checks were added (no try/except blocks), pass if not checks: return { 'passed': True, - 'checks': [{'name': 'Error handling check', 'passed': True, 'message': 'No checks applicable'}], + 'checks': [{'name': 'Error handling', 'passed': True, 'message': 'No try/except blocks detected (not applicable)'}], 'score': 100, 'standard': 'ERROR_HANDLING' } @@ -154,8 +89,6 @@ def check_module(module_path: str, bypass_rules: list | None = None) -> Dict: passed_checks = sum(1 for check in checks if check['passed']) total_checks = len(checks) score = int((passed_checks / total_checks * 100)) if total_checks > 0 else 0 - - # Overall pass if score >= 75% overall_passed = score >= 75 json_handler.log_operation("check_completed", {"file": str(module_path), "score": score, "standard": "error_handling"}) @@ -167,173 +100,60 @@ def check_module(module_path: str, bypass_rules: list | None = None) -> Dict: } -def check_module_has_prax(content: str, file_path: str, bypass_rules: list | None = None) -> Dict: - """ - Check that modules import Prax logger - - Modules MUST import Prax for business logging - """ - has_prax_import = ( - 'from aipass.prax import logger' in content - or 'from aipass.prax import' in content and 'logger' in content - or 'from aipass.prax.apps.modules.logger import system_logger' in content - ) - - if has_prax_import: - return { - 'name': 'Module Prax import', - 'passed': True, - 'message': 'Module imports Prax logger (required for business logging)' - } - else: - # Check if bypassed (whole file bypass for this check) - if is_bypassed(file_path, 'error_handling', None, bypass_rules): - return { - 'name': 'Module Prax import', - 'passed': True, - 'message': 'Module Prax import check bypassed' - } - return { - 'name': 'Module Prax import', - 'passed': False, - 'message': 'Module MUST import Prax: from aipass.prax import logger' - } +def _is_silent_except(lines: List[str], pass_index: int, pass_line: str) -> bool: + pass_indent = len(pass_line) - len(pass_line.lstrip()) + for j in range(pass_index, min(pass_index + 3, len(lines))): + next_line = lines[j].strip() + is_pass_line = next_line == 'pass' or next_line.startswith('pass ') or next_line.startswith('pass#') + if next_line and not is_pass_line: + if lines[j].startswith(' ') and len(lines[j]) - len(lines[j].lstrip()) > pass_indent: + return False + break + return True -def check_handler_no_stdlib_logging(content: str, file_path: str, bypass_rules: list | None = None) -> Dict: - """ - Check that handlers do NOT use stdlib ``logging.get`` ``Logger()``. +def check_error_handling(content: str, lines: List[str], module_path: str = "") -> Optional[Dict]: + """Check for proper error handling patterns""" + try_count = content.count('try:') - Handlers should use Prax system_logger instead. stdlib logging - creates blind spots invisible to Prax monitor. - """ - lines = content.split('\n') - stdlib_lines = [] + if try_count == 0: + return None + silent_failures = [] in_docstring = False - for i, line in enumerate(lines, 1): + in_except = False + except_line = 0 + + for i, line in enumerate(lines): stripped = line.strip() - - # Track docstrings - if '"""' in line or "'''" in line: - in_docstring = not in_docstring + if stripped.startswith('"""') or stripped.startswith("'''"): + quote = '"""' if stripped.startswith('"""') else "'''" + if stripped.count(quote) == 2 and len(stripped) > len(quote) * 2: + pass + else: + in_docstring = not in_docstring + if in_docstring: continue - - # Skip if in docstring or comment - if in_docstring or stripped.startswith('#'): + if 'except' in stripped and ':' in stripped: + in_except = True + except_line = i continue + if in_except: + if stripped == 'pass' or stripped.startswith('pass ') or stripped.startswith('pass#'): + if _is_silent_except(lines, i, line): + silent_failures.append(f"line {except_line}") + if line.strip() and not line.startswith(' ') and not line.startswith('\t'): + in_except = False - # Check for stdlib logging.getLogger usage - if re.search(r'logging\.getLogger\s*\(', line): - if not is_bypassed(file_path, 'error_handling', i, bypass_rules): - stdlib_lines.append(i) - - if stdlib_lines: + if silent_failures: return { - 'name': 'Handler stdlib logging', + 'name': 'Error handling', 'passed': False, - 'message': f'Handler uses stdlib logging.get' f'Logger() on lines {stdlib_lines[:3]} — use Prax system_logger instead' - } - else: - return { - 'name': 'Handler stdlib logging', - 'passed': True, - 'message': 'Handler correctly avoids stdlib logging.get' 'Logger()' + 'message': f'Silent failure detected (except: pass) in {Path(module_path).name if module_path else "file"} at {silent_failures[0]} - errors should log/return' } - -def check_module_error_logging(content: str) -> Dict: - """ - Check that modules have Prax logger available for error logging. - - Entry point @track_operation handles exception catching. - Modules just need Prax import (checked separately). - This check passes if Prax is imported - modules CAN log but aren't required to. - """ - has_prax_import = 'from aipass.prax import logger' in content or ( - 'from aipass.prax import' in content and 'logger' in content - ) or 'from aipass.prax.apps.modules.logger import system_logger' in content - - if has_prax_import: - return { - 'name': 'Module error logging', - 'passed': True, - 'message': 'Module has Prax logger available (entry point handles exceptions)' - } - else: - # No Prax import - this is caught by check_module_has_prax - # Still pass here to avoid double-flagging - return { - 'name': 'Module error logging', - 'passed': True, - 'message': 'Module error logging deferred to Prax import check' - } - - -def _matches_user_input_pattern(line_lower: str, user_input_patterns: list) -> bool: - for pattern in user_input_patterns: - if re.search(pattern, line_lower): - return True - return False - - -def check_error_vs_warning_usage(lines: List[str], file_path: str, bypass_rules: list | None = None) -> Dict: - """ - Check that logger.error() is used for system failures, not user input validation. - - User input validation (should be WARNING): - - "not found", "invalid", "required", "missing", "does not exist" - - System failures (should be ERROR): - - File I/O errors, crashes, dependency failures - """ - # Patterns that indicate user input validation (should be warning, not error) - user_input_patterns = [ - r'not\s+found', - r'invalid', - r'\brequired\b', - r'\bmissing\b', - r'does\s+not\s+exist', - r'unable\s+to\s+find', - r'no\s+such', - r'doesn\'t\s+exist', - r'not\s+exist', - ] - - violations = [] - - in_docstring = False - for i, line in enumerate(lines, 1): - stripped = line.strip() - - # Track docstrings - if '"""' in line or "'''" in line: - in_docstring = not in_docstring - continue - - # Skip if in docstring or comment - if in_docstring or stripped.startswith('#'): - continue - - # Look for logger.error() calls - if re.search(r'logger\.error\s*\(', line): - # Check if the message contains user input patterns - line_lower = line.lower() - if _matches_user_input_pattern(line_lower, user_input_patterns): - if not is_bypassed(file_path, 'error_handling', i, bypass_rules): - violations.append(i) - - if violations: - return { - 'name': 'ERROR vs WARNING usage', - 'passed': False, - 'message': f'logger.error() used for user input validation on lines {violations[:5]} - use logger.warning() instead' - } - else: - return { - 'name': 'ERROR vs WARNING usage', - 'passed': True, - 'message': 'logger.error() correctly used for system failures only' - } - - + return { + 'name': 'Error handling', + 'passed': True, + 'message': f'Error handling present ({try_count} try/except blocks with proper handling)' + } diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling_content.py b/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling_content.py index a56c888a..96d21b51 100644 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling_content.py +++ b/src/aipass/seedgo/apps/handlers/aipass_standards/error_handling_content.py @@ -2,164 +2,92 @@ # Name: error_handling_content.py # Description: Error Handling Standards Content Handler # Version: 1.0.0 -# Created: 2026-03-05 -# Modified: 2026-03-05 +# Created: 2026-03-27 +# Modified: 2026-03-27 # ============================================= """ Error Handling Standards Content Handler -Provides formatted error handling standards content (3-tier architecture). +Provides formatted error handling standards content. Module orchestrates, handler implements. """ from aipass.seedgo.apps.handlers.json import json_handler + def get_error_handling_standards() -> str: - """Return formatted error handling standards content with Rich markup + """Return formatted error_handling standards content with Rich markup Returns: str: Formatted standards text with Rich styling """ lines = [ - "[bold red]3-TIER LOGGING ARCHITECTURE[/bold red]", + "[bold cyan]CORE PRINCIPLE:[/bold cyan]", + " Errors must tell the truth. Silent failures hide bugs and erode", + " trust in the codebase. Every except block must handle the error", + " meaningfully — log it, return it, or re-raise it.", "", - "[yellow]CORE PRINCIPLE:[/yellow] Logging responsibility follows architectural hierarchy", + "[bold cyan]WHAT IT CHECKS:[/bold cyan]", + " File-level analysis of try/except patterns:", "", - "[bold cyan]TIER 1: ENTRY POINTS[/bold cyan] (flow.py, seedgo.py, etc.)", - " [dim]Prax Import:[/dim] YES (minimal)", - " [dim]Logging Scope:[/dim] Operational only (discovery, routing, help)", - " [red]✗ NO business error logging[/red]", + " [yellow]Detection:[/yellow]", + " Scans all Python files for [dim]try/except[/dim] blocks", + " Flags silent failures: [dim]except: pass[/dim] or [dim]except Exception: pass[/dim]", + " Skips files with no try/except (not applicable)", "", - "[bold cyan]TIER 2: MODULES[/bold cyan] (apps/modules/*.py)", - " [dim]Prax Import:[/dim] [green]YES (REQUIRED)[/green]", - " [dim]Logging Scope:[/dim] [green]ALL BUSINESS LOGGING[/green]", - " [green]✓ Log ALL errors: logger.error(f'[{MODULE_NAME}] {msg}')[/green]", - " [green]✓ Catch exceptions from handlers[/green]", - " [green]✓ Provide context for debugging[/green]", - " [green]✓ Return bool to entry point[/green]", + "[bold cyan]VIOLATIONS:[/bold cyan]", "", - "[bold cyan]TIER 3: HANDLERS[/bold cyan] (apps/handlers/**/*.py)", - " [dim]Prax Import:[/dim] [green]ALLOWED[/green]", - " [dim]Logging Scope:[/dim] [green]info, warning, error[/green]", - " [green]✓ logger.info() — operational visibility[/green]", - " [green]✓ logger.warning() — non-critical issues[/green]", - " [green]✓ logger.error() — error context at point of failure[/green]", - " [green]✓ Return status dicts or raise exceptions[/green]", - " [green]✓ Testable in isolation[/green]", + " [red]Fail:[/red] Silent failure detected — [dim]except: pass[/dim] with no", + " logging, return, or re-raise", "", - "─" * 70, + " [green]Pass:[/green] All except blocks have meaningful handling", + " (logging, return values, re-raise, or other statements)", "", - "[bold yellow]HANDLER RETURN PATTERNS:[/bold yellow]", - "", - "[dim]Option 1: Status Dict[/dim]", - " return {'success': True, 'data': result, 'error': None}", - " return {'success': False, 'data': None, 'error': 'Error message'}", - "", - "[dim]Option 2: Raise Exceptions[/dim]", - " raise ValueError('Invalid input')", - " raise FileNotFoundError('Config missing')", - "", - "─" * 70, - "", - "[bold yellow]MODULE ERROR HANDLING PATTERN:[/bold yellow]", - "", - " [dim]MODULE_NAME = \"module_name\"[/dim]", + "[bold cyan]CODE EXAMPLES:[/bold cyan]", "", + " [green]Good:[/green]", " [dim]try:[/dim]", - " [dim]result = handler_function(**args)[/dim]", - " [dim]if result['success']:[/dim]", - " [dim]logger.info(f\"[{MODULE_NAME}] Success\")[/dim]", - " [dim]else:[/dim]", - " [dim]logger.error(f\"[{MODULE_NAME}] {result['error']}\")[/dim]", + " [dim]result = api_call()[/dim]", " [dim]except Exception as e:[/dim]", - " [dim]logger.error(f\"[{MODULE_NAME}] Error: {e}\")[/dim]", + " [dim]logger.error(f'API call failed: {e}')[/dim]", + " [dim]return {'success': False, 'error': str(e)}[/dim]", "", - "─" * 70, + " [red]Bad:[/red]", + " [dim]try:[/dim]", + " [dim]result = api_call()[/dim]", + " [dim]except:[/dim]", + " [dim]pass # Silent failure — error swallowed[/dim]", "", - "[bold yellow]LOG LEVEL GUIDELINES: ERROR vs WARNING[/bold yellow]", + "[bold cyan]PHILOSOPHY:[/bold cyan]", "", - "[dim]Core Distinction:[/dim] ERROR triggers Prax escalation, WARNING does not", + " [yellow]Fix error handling BEFORE fixing bugs.[/yellow]", + " If errors lie, you can't debug effectively.", + " Honest errors → faster debugging → reliable code.", "", - "[red]logger.error()[/red] → [bold]System failures[/bold]", - " File I/O errors, crashes, dependency failures, unexpected exceptions", + " The debug cycle:", + " [dim]1. Fix error handling — make errors tell the truth[/dim]", + " [dim]2. Then fix the actual bug[/dim]", + " [dim]3. See clean pass with honest outputs[/dim]", "", - "[yellow]logger.warning()[/yellow] → [bold]User input issues[/bold]", - " 'Plan not found', 'Field required', 'Invalid format', 'Already exists'", + "[bold cyan]SCORING:[/bold cyan]", "", - "[dim]Pattern:[/dim]", - " [red]# System failure → ERROR (Prax escalates)[/red]", - " [dim]logger.error(f\"[{MODULE_NAME}] Failed to write file: {e}\")[/dim]", + " Score = (passed_checks / total_checks) * 100", + " Overall pass threshold: [yellow]75%[/yellow]", + " No try/except blocks: [green]100%[/green] (not applicable)", "", - " [yellow]# User input issue → WARNING (no escalation)[/yellow]", - " [dim]logger.warning(f\"[{MODULE_NAME}] Plan {plan_id} not found\")[/dim]", + "[yellow]SCOPE:[/yellow]", + " AUDIT_SCOPE = [bold]all_files[/bold]", + " Runs on every Python file. Entry point: [dim]check_module()[/dim]", "", - "[green]✓ CLI can still show 'ERROR' text to user (feedback)[/green]", - "[green]✓ System logs use WARNING level (no Prax escalation)[/green]", - "", - "─" * 70, - "", - "[bold yellow]WHY THIS PATTERN:[/bold yellow]", - "", - "[green]✓ Scalability:[/green] 17 branches × 50+ handlers = 850 files to manage", - "[green]✓ Auditability:[/green] Check modules/ only - all logging in one place", - "[green]✓ Testability:[/green] Automated scans verify compliance", - "[green]✓ Consistency:[/green] Single pattern across entire ecosystem", - "", - "─" * 70, - "", - "[bold yellow]VALIDATION RULES:[/bold yellow]", - "", - "[dim]# All modules MUST import Prax[/dim]", - "[dim]grep -r \"from aipass.prax\" apps/modules/*.py[/dim]", - "", - "[dim]# Handlers MAY import Prax and use all log levels[/dim]", - "[dim]grep -r \"from aipass.prax\" apps/handlers/**/*.py # Allowed[/dim]", - "", - "[dim]# Handlers MUST NOT use stdlib logging.getLogger()[/dim]", - "[dim]grep -r \"logging.getLogger\" apps/handlers/**/*.py # Should find NOTHING[/dim]", - "", - "─" * 70, + "[bold cyan]BYPASS:[/bold cyan]", + " Via [dim].seedgo/bypass.json[/dim] — supports standard-level,", + " file-level, and line-level bypass rules", "", "[bold cyan]REFERENCE:[/bold cyan]", - " [dim]See: seedgo standards pack (error_handling)[/dim]", - " [dim]See: src/aipass/flow/apps/modules/ (working example)[/dim]", - " [dim]See: src/aipass/seedgo/apps/standards/aipass/modules/ (reference implementation)[/dim]", - "", - "[bold]Decision:[/bold] 3-Tier approved 2025-11-21, ERROR/WARNING 2026-01-31", - "[bold]Status:[/bold] Active - All 18 branches", - "", - "─" * 70, - "", - "[bold magenta]SERVICE LOGGING STANDARDS[/bold magenta]", - "", - "[yellow]For services with user-facing interactions (bridges, APIs, bots):[/yellow]", - "", - "[bold cyan]1. LOG MEANINGFUL CONTENT[/bold cyan]", - " [green]✓ 'Received from user X: hello' + 'Sent: Hi!'[/green]", - " [red]✗ 'Message received' + 'Response sent'[/red]", - "", - "[bold cyan]2. AUDIT TRAILS REQUIRED[/bold cyan]", - " • Chat messages and responses", - " • API requests and responses", - " • User interactions with context", - "", - "[bold cyan]3. LOG LOCATIONS[/bold cyan]", - " [dim]~/system_logs/[/dim] → Prax operational logs", - " [dim]/logs/[/dim] → Service content logs", - " [dim]/logs/audit/[/dim] → Interaction audit trails", - "", - "[bold cyan]4. CONTENT LOG FORMAT[/bold cyan]", - " [dim]{[/dim]", - " [dim]'timestamp': '2026-02-04T10:30:00Z',[/dim]", - " [dim]'direction': 'inbound',[/dim]", - " [dim]'user': 'user_id',[/dim]", - " [dim]'content': 'message text',[/dim]", - " [dim]'response': 'bot response',[/dim]", - " [dim]'metadata': {...}[/dim]", - " [dim]}[/dim]", - "", - "[bold]Added:[/bold] 2026-02-04 - Service logging for user-facing interactions", + " [dim]Checker: error_handling_check.py[/dim]", + " [dim]Standard label: ERROR_HANDLING[/dim]", + " [dim]Previously: testing_check.py (renamed for clarity)[/dim]", ] json_handler.log_operation("standard_content_queried", {"standard": "error_handling"}) diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/test_coverage.md b/src/aipass/seedgo/apps/handlers/aipass_standards/test_coverage.md deleted file mode 100644 index 5ca46731..00000000 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/test_coverage.md +++ /dev/null @@ -1,131 +0,0 @@ -# Test Coverage Standards -**Status:** Draft v1 -**Date:** 2026-03-22 - ---- - -## What It Is - -The test coverage standard evaluates how well a branch's modules and handlers are exercised by test files. It discovers test files, counts pytest-style test functions, maps which modules they cover via import patterns, and calculates a coverage percentage. - ---- - -## Why It Matters - -Untested code is unverified code. Without tests, changes can silently break functionality, regressions go unnoticed, and confidence in the codebase erodes. Test coverage tracking provides visibility into what is tested and what is not, making it clear where investment is needed. - ---- - -## What the Checker Scans For - -This is a **branch-level** checker that runs once per branch (not per file). It operates in four phases: - -### Phase 1: Discovery - -Finds test files by looking in: -- `{branch}/tests/` directory (recursive scan) -- Any file matching `test_*.py` or `*_test.py` elsewhere in the branch - -Skips: `__init__.py`, `conftest.py`, `__pycache__`, and directories in the skip list. - -### Phase 2: Analysis - -For each test file: -- Counts pytest-style test functions (`def test_*` and `async def test_*`) -- Maps tested modules via import patterns: - - `from aipass..apps.modules. import ...` - - `from aipass..apps.handlers. import ...` - - `import aipass..apps.modules.` - -### Phase 3: Testable Module Collection - -Collects module names from: -- `apps/modules/*.py` -- file stem becomes module name (e.g., `runner.py` -> `runner`) -- `apps/handlers/*.py` -- file stem becomes module name -- `apps/handlers/subdir/` -- directory name if it contains `.py` files - -### Phase 4: Coverage Calculation - -``` -coverage = covered_modules / total_testable_modules * 100 -``` - -Where `covered_modules` is the intersection of tested modules (from imports) and all testable modules. - ---- - -## Three Checks - -1. **Test files** -- do any test files exist? -2. **Test functions** -- are there `def test_*` functions? -3. **Module coverage** -- what percentage of modules have test coverage? - - Threshold: **25%** (lenient -- most branches have no tests yet) - ---- - -## Code Examples - -### Violation - -A branch with `apps/modules/runner.py` and `apps/handlers/audit/` but no `tests/` directory and no `test_*.py` files anywhere. - -### Fix - -```python -# tests/test_runner.py -from aipass.seedgo.apps.modules import runner - - -def test_runner_executes(): - result = runner.run("check") - assert result is not None - - -def test_runner_handles_missing_target(): - result = runner.run("") - assert result["passed"] is False -``` - ---- - -## Scoring - -- **Scope:** `AUDIT_SCOPE = "branch_level"` -- runs once per branch via `check_branch()` -- **Score formula:** `covered_modules / total_modules * 100` -- **No testable modules:** Score = 100 (nothing to test) -- **Overall pass threshold:** 75% - ---- - -## Skipped Directories - -The following directories are excluded from test file discovery: - -`__pycache__`, `.archive`, `.mypy_cache`, `.ruff_cache`, `.pytest_cache`, `.venv`, `venv`, `node_modules`, `.git`, `site-packages`, `logs`, `tools`, `.trinity`, `.aipass`, `.ai_mail.local`, `.spawn`, `backups`, `reports`, `docs`, `.sorting_unprocessed` - ---- - -## Bypass - -Bypass rules are configured in `.seedgo/bypass.json`. Supports: - -- **Standard-level bypass:** Skip the entire `test_coverage` standard for a branch -- **File-level bypass:** Match by file path substring -- **Line-level bypass:** Skip specific lines (less common for branch-level checks) - -Example bypass rule: -```json -{ - "standard": "test_coverage", - "file": "experimental_branch" -} -``` - ---- - -## Reference - -- **Checker:** `test_coverage_check.py` -- **Scope:** `branch_level` -- **Entry point:** `check_branch(branch_path, bypass_rules)` -- **Standard label:** `TEST_COVERAGE` diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/test_coverage_check.py b/src/aipass/seedgo/apps/handlers/aipass_standards/test_coverage_check.py deleted file mode 100644 index 1db1f5b8..00000000 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/test_coverage_check.py +++ /dev/null @@ -1,371 +0,0 @@ -# =================== AIPass ==================== -# Name: test_coverage_check.py -# Description: Test Coverage Standards Checker Handler -# Version: 1.0.0 -# Created: 2026-03-22 -# Modified: 2026-03-22 -# ============================================= - -""" -Test Coverage Standards Checker Handler - -Branch-level checker that evaluates test coverage for a branch by: -- Discovering test files (tests/ directory and scattered test_*.py / *_test.py) -- Counting pytest-style test functions (def test_*) -- Mapping tested modules via import patterns -- Calculating module coverage (covered / total testable modules) - -Extracted from devpulse test_scanner_v1 and wrapped as a seedgo checker. -""" - -import re -from pathlib import Path - -from aipass.prax import logger -from aipass.seedgo.apps.handlers.json import json_handler - -AUDIT_SCOPE = "branch_level" - -# -- Directories to skip when scanning ---------------------------------------- -SKIP_DIRS: set[str] = { - "__pycache__", ".archive", ".mypy_cache", ".ruff_cache", - ".pytest_cache", ".venv", "venv", "node_modules", ".git", - "site-packages", "logs", "tools", ".trinity", ".aipass", - ".ai_mail.local", ".spawn", "backups", "reports", "docs", - ".sorting_unprocessed", -} - -# -- Test function pattern ---------------------------------------------------- -RE_TEST_FUNC = re.compile(r"^\s*(?:async\s+)?def\s+(test_\w+)", re.MULTILINE) - -# -- Import patterns for mapping tests to modules ---------------------------- -RE_IMPORT_FROM = re.compile( - r"from\s+(?:aipass\.)?\w+\.apps\.(?:modules|handlers)[./]?([\w.]*)\s+import" -) -RE_IMPORT_DIRECT = re.compile( - r"import\s+(?:aipass\.)?\w+\.apps\.(?:modules|handlers)[./]?([\w.]*)" -) - - -# ============================================= -# BYPASS HELPER -# ============================================= - -def is_bypassed(file_path: str, standard: str, line: int | None = None, bypass_rules: list | None = None) -> bool: - """Check if a violation should be bypassed.""" - if not bypass_rules: - return False - for rule in bypass_rules: - if rule.get("standard") and rule.get("standard") != standard: - continue - rule_file = rule.get("file", "") - if rule_file and rule_file not in file_path: - continue - rule_lines = rule.get("lines", []) - if rule_lines and line is not None and line not in rule_lines: - continue - return True - return False - - -# ============================================= -# FILE HELPERS -# ============================================= - -def _read_file_safe(path: Path) -> str: - """Read a file, returning empty string on any error.""" - try: - return path.read_text(encoding="utf-8") - except (OSError, UnicodeDecodeError): - logger.info("Cannot read %s for test coverage analysis", path) - return "" - - -def _should_skip_dir(name: str) -> bool: - """Check if a directory name should be skipped.""" - return name in SKIP_DIRS or name.startswith(".") - - -# ============================================= -# PHASE 1: DISCOVERY -# ============================================= - -def _find_test_files(branch_path: Path) -> list[Path]: - """Find all test files for a branch. - - Looks in: - - {branch_path}/tests/ (recursive) - - Any file matching test_*.py or *_test.py elsewhere in the branch - """ - test_files: list[Path] = [] - seen: set[Path] = set() - - # 1. Standard tests/ directory - tests_dir = branch_path / "tests" - if tests_dir.is_dir(): - for py_file in sorted(tests_dir.rglob("*.py")): - if py_file.name in ("__init__.py", "conftest.py"): - continue - if "__pycache__" in py_file.parts: - continue - resolved = py_file.resolve() - if resolved not in seen: - seen.add(resolved) - test_files.append(py_file) - - # 2. Scattered test_*.py or *_test.py anywhere in the branch - for py_file in sorted(branch_path.rglob("*.py")): - if any(_should_skip_dir(part) for part in py_file.relative_to(branch_path).parts): - continue - if py_file.name in ("__init__.py", "conftest.py"): - continue - if py_file.name.startswith("test_") or py_file.name.endswith("_test.py"): - resolved = py_file.resolve() - if resolved not in seen: - seen.add(resolved) - test_files.append(py_file) - - return test_files - - -# ============================================= -# PHASE 2: ANALYZE TEST FILES -# ============================================= - -def _analyze_test_file(test_file: Path) -> dict: - """Analyze a single test file for test functions and module coverage. - - Returns: - dict with keys: path, test_count, test_names, tested_modules - """ - source = _read_file_safe(test_file) - info: dict = { - "path": test_file, - "test_count": 0, - "test_names": [], - "tested_modules": set(), - } - - if not source: - return info - - # Count test functions - for match in RE_TEST_FUNC.finditer(source): - info["test_names"].append(match.group(1)) - info["test_count"] = len(info["test_names"]) - - # Find which modules this test file covers via imports - for match in RE_IMPORT_FROM.finditer(source): - sub_path = match.group(1) - if sub_path: - first_segment = sub_path.split(".")[0] - info["tested_modules"].add(first_segment) - - for match in RE_IMPORT_DIRECT.finditer(source): - sub_path = match.group(1) - if sub_path: - first_segment = sub_path.split(".")[0] - info["tested_modules"].add(first_segment) - - return info - - -# ============================================= -# PHASE 3: COLLECT TESTABLE MODULES -# ============================================= - -def _collect_testable_modules(branch_path: Path) -> set[str]: - """Collect module names from apps/modules/ and apps/handlers/. - - Returns set of module names: - - apps/modules/*.py -> file stem (e.g. "runner") - - apps/handlers/*.py -> file stem (e.g. "audit") - - apps/handlers/subdir/ -> directory name if it contains .py files - """ - modules: set[str] = set() - apps_dir = branch_path / "apps" - if not apps_dir.is_dir(): - return modules - - # apps/modules/ -- flat .py files - modules_dir = apps_dir / "modules" - if modules_dir.is_dir(): - for item in sorted(modules_dir.iterdir()): - if item.is_file() and item.suffix == ".py" and item.name != "__init__.py": - modules.add(item.stem) - - # apps/handlers/ -- flat .py files OR subdirectories with .py files - handlers_dir = apps_dir / "handlers" - if handlers_dir.is_dir(): - for item in sorted(handlers_dir.iterdir()): - if _should_skip_dir(item.name): - continue - if item.is_dir() and item.name != "__pycache__": - has_py = any( - f.suffix == ".py" and f.name != "__init__.py" - for f in item.iterdir() - if f.is_file() - ) - if has_py: - modules.add(item.name) - elif item.is_file() and item.suffix == ".py" and item.name != "__init__.py": - modules.add(item.stem) - - return modules - - -# ============================================= -# PHASE 4: BRANCH-LEVEL CHECK -# ============================================= - -def check_branch(branch_path: str, bypass_rules: list | None = None) -> dict: - """Run test coverage analysis on a branch. - - Args: - branch_path: Path to branch root directory - bypass_rules: Optional list of bypass rules - - Returns: - dict: {passed, score, checks, standard: 'TEST_COVERAGE'} - """ - checks: list[dict] = [] - bp = Path(branch_path) - - # Check if entire standard is bypassed - if is_bypassed(branch_path, "test_coverage", bypass_rules=bypass_rules): - return { - "passed": True, - "checks": [ - { - "name": "Bypassed", - "passed": True, - "message": "Standard bypassed via .seedgo/bypass.json", - } - ], - "score": 100, - "standard": "TEST_COVERAGE", - } - - # Validate branch path exists - if not bp.is_dir(): - return { - "passed": False, - "checks": [ - { - "name": "Branch exists", - "passed": False, - "message": f"Branch directory not found: {branch_path}", - } - ], - "score": 0, - "standard": "TEST_COVERAGE", - } - - # Phase 1: Find test files - test_files = _find_test_files(bp) - - # Phase 2: Analyze each test file - total_tests = 0 - tested_modules: set[str] = set() - for tf in test_files: - info = _analyze_test_file(tf) - total_tests += info["test_count"] - tested_modules.update(info["tested_modules"]) - - # Clear tested modules if no actual test functions found - if total_tests == 0: - tested_modules = set() - - # Phase 3: Collect all testable modules - all_modules = _collect_testable_modules(bp) - total_modules = len(all_modules) - - # Phase 4: Calculate coverage - if total_modules > 0: - covered_count = len(tested_modules & all_modules) - coverage_pct = (covered_count / total_modules) * 100 - else: - covered_count = 0 - coverage_pct = 0.0 - - # -- Check 1: Test files exist -- - if test_files: - checks.append({ - "name": "Test files", - "passed": True, - "message": f"Found {len(test_files)} test file(s)", - }) - else: - checks.append({ - "name": "Test files", - "passed": False, - "message": "No test files found (expected tests/ dir or test_*.py files)", - }) - - # -- Check 2: Test functions -- - if total_tests > 0: - checks.append({ - "name": "Test functions", - "passed": True, - "message": f"Found {total_tests} test function(s)", - }) - else: - checks.append({ - "name": "Test functions", - "passed": False, - "message": "No test functions found (expected def test_* functions)", - }) - - # -- Check 3: Module coverage -- - # Lenient threshold: 25% -- most branches have no tests yet - coverage_threshold = 25 - if total_modules == 0: - checks.append({ - "name": "Module coverage", - "passed": True, - "message": "No testable modules found (nothing to test)", - }) - elif coverage_pct >= coverage_threshold: - checks.append({ - "name": "Module coverage", - "passed": True, - "message": f"{covered_count}/{total_modules} modules covered ({coverage_pct:.0f}%)", - }) - else: - checks.append({ - "name": "Module coverage", - "passed": False, - "message": ( - f"{covered_count}/{total_modules} modules covered ({coverage_pct:.0f}%) " - f"-- below {coverage_threshold}% threshold" - ), - }) - - # Calculate score - # If branch has 0 testable modules, score = 100 (nothing to test) - if total_modules == 0: - score = 100 - else: - score = int((covered_count / total_modules) * 100) - - # Overall pass at 75% score threshold - overall_passed = score >= 75 - - json_handler.log_operation( - "check_completed", - { - "branch": branch_path, - "score": score, - "standard": "test_coverage", - "total_tests": total_tests, - "covered_modules": covered_count, - "total_modules": total_modules, - }, - ) - - return { - "passed": overall_passed, - "score": score, - "checks": checks, - "standard": "TEST_COVERAGE", - } diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/test_coverage_content.py b/src/aipass/seedgo/apps/handlers/aipass_standards/test_coverage_content.py deleted file mode 100644 index 6920096f..00000000 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/test_coverage_content.py +++ /dev/null @@ -1,106 +0,0 @@ -# =================== AIPass ==================== -# Name: test_coverage_content.py -# Description: Test Coverage Standards Content Handler -# Version: 1.0.0 -# Created: 2026-03-22 -# Modified: 2026-03-22 -# ============================================= - -""" -Test Coverage Standards Content Handler - -Provides formatted Test Coverage standards content. -Module orchestrates, handler implements. -""" - -from aipass.seedgo.apps.handlers.json import json_handler - - -def get_test_coverage_standards() -> str: - """Return formatted test_coverage standards content with Rich markup - - Returns: - str: Formatted standards text with Rich styling - """ - lines = [ - "[bold cyan]CORE PRINCIPLE:[/bold cyan]", - " Every branch should have tests. Test coverage measures how many", - " of a branch's modules and handlers are exercised by test files.", - " Untested code is unverified code.", - "", - "[bold cyan]WHAT IT CHECKS:[/bold cyan]", - " Branch-level analysis in four phases:", - "", - " [yellow]Phase 1 -- Discovery:[/yellow]", - " Finds test files in [dim]tests/[/dim] directory (recursive) and", - " scattered [dim]test_*.py[/dim] / [dim]*_test.py[/dim] files elsewhere", - " Skips: __init__.py, conftest.py, __pycache__", - "", - " [yellow]Phase 2 -- Analysis:[/yellow]", - " Counts pytest-style test functions ([dim]def test_*[/dim] and", - " [dim]async def test_*[/dim]) in each test file", - " Maps tested modules via import patterns:", - " [dim]from aipass..apps.modules. import ...[/dim]", - " [dim]from aipass..apps.handlers. import ...[/dim]", - "", - " [yellow]Phase 3 -- Testable modules:[/yellow]", - " Collects module names from [dim]apps/modules/*.py[/dim] and", - " [dim]apps/handlers/*.py[/dim] (or subdirectories with .py files)", - "", - " [yellow]Phase 4 -- Coverage calculation:[/yellow]", - " [dim]coverage = covered_modules / total_testable_modules * 100[/dim]", - "", - "[bold cyan]THREE CHECKS:[/bold cyan]", - "", - " [bold]1. Test files[/bold] -- do any test files exist?", - " [bold]2. Test functions[/bold] -- are there [dim]def test_*[/dim] functions?", - " [bold]3. Module coverage[/bold] -- what % of modules are covered?", - " Threshold: [yellow]25%[/yellow] (lenient -- most branches have no tests yet)", - "", - "[bold cyan]VIOLATIONS:[/bold cyan]", - "", - " [red]Fail:[/red] No [dim]tests/[/dim] directory and no test_*.py files found", - " [red]Fail:[/red] Test files exist but contain no [dim]def test_*[/dim] functions", - " [red]Fail:[/red] Module coverage below 25% threshold", - "", - "[bold cyan]HOW TO FIX:[/bold cyan]", - "", - " 1. Create a [dim]tests/[/dim] directory in your branch", - " 2. Add test files with pytest-style test functions:", - "", - " [green]Good:[/green]", - " [dim]# tests/test_runner.py[/dim]", - " [dim]from aipass.seedgo.apps.modules import runner[/dim]", - " [dim][/dim]", - " [dim]def test_runner_executes():[/dim]", - " [dim] result = runner.run(\"check\")[/dim]", - " [dim] assert result is not None[/dim]", - "", - " 3. Import the modules you are testing so the coverage mapper", - " can detect which modules your tests cover", - "", - "[yellow]SCOPE:[/yellow]", - " AUDIT_SCOPE = [bold]branch_level[/bold]", - " Runs once per branch (not per file). Entry point: [dim]check_branch()[/dim]", - "", - "[bold cyan]SCORING:[/bold cyan]", - " Score = [dim]covered_modules / total_modules * 100[/dim]", - " If branch has 0 testable modules: score = [green]100[/green]", - " Overall pass threshold: [yellow]75%[/yellow]", - "", - "[bold cyan]BYPASS:[/bold cyan]", - " Via [dim].seedgo/bypass.json[/dim] -- supports standard-level and", - " file-level bypass rules", - "", - "[bold cyan]SKIPPED DIRECTORIES:[/bold cyan]", - " __pycache__, .archive, .mypy_cache, .ruff_cache, .pytest_cache,", - " .venv, venv, node_modules, .git, site-packages, logs, tools,", - " .trinity, .aipass, .ai_mail.local, .spawn, backups, reports, docs", - "", - "[bold cyan]REFERENCE:[/bold cyan]", - " [dim]See: seedgo standards pack (test_coverage)[/dim]", - " [dim]Checker: test_coverage_check.py[/dim]", - ] - - json_handler.log_operation("standard_content_queried", {"standard": "test_coverage"}) - return "\n".join(lines) diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality.md b/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality.md index f3816e14..ae70dda4 100644 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality.md +++ b/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality.md @@ -1,18 +1,20 @@ # Test Quality Standards -**Status:** v3.0 — 10 standard categories (48 items) -**Date:** 2026-03-24 +**Status:** v4.0 — 11 categories (51 items), consolidated +**Date:** 2026-03-27 --- ## What It Is -The test quality standard evaluates whether a branch's test files cover 10 standard test categories (48 total items). It scans ALL `test_*.py` files and `conftest.py` in a branch's `tests/` directory. This is a static analysis check; it does not run pytest, only inspects test file contents for detection patterns. +The test quality standard evaluates whether a branch's test files cover 11 standard categories (51 total items): 10 pattern-based categories plus module coverage. It scans ALL `test_*.py` files and `conftest.py` in a branch's `tests/` directory. This is a static analysis check; it does not run pytest, only inspects test file contents. + +**v4.0 consolidation:** The former `test_coverage` standard (import-based module coverage) is now category 11 within this checker. One unified test standard instead of two. --- ## Why It Matters -Standard test categories ensure every branch tests its shared infrastructure (json_handler, CLI routing), error handling, contracts, and fixtures consistently. Coverage breadth across categories means reliable, predictable behavior across the ecosystem. +Standard test categories ensure every branch tests its shared infrastructure (json_handler, CLI routing), error handling, contracts, and fixtures consistently. Module coverage ensures test files actually import and exercise the branch's modules. Coverage breadth across categories means reliable, predictable behavior across the ecosystem. --- @@ -106,15 +108,26 @@ Standard test categories ensure every branch tests its shared infrastructure (js | sys_modules_mock | `sys.modules` | | reimport_after_mock | `importlib.reload`, `reload(` | +### 11. Module Coverage (3 items) +| Item | What It Checks | +|------|---------------| +| test_files_exist | At least one test file found (tests/ or scattered test_*.py) | +| test_functions_exist | At least one `def test_*` function found | +| module_coverage_25pct | >= 25% of modules in apps/modules/ and apps/handlers/ are covered via imports | + +Module coverage uses import-based mapping: +- `from aipass..apps.modules. import ...` -> covers module `` +- `from aipass..apps.handlers. import ...` -> covers handler `` + --- ## Scoring -Score = (items_covered / 48) x 100 +Score = (items_covered / 51) x 100 -- **Overall pass threshold:** 75% (36+ of 48 items) +- **Overall pass threshold:** 75% (39+ of 51 items) - **No test files:** 0% -- **All 48 items covered:** 100% +- **All 51 items covered:** 100% --- @@ -146,3 +159,10 @@ Bypass rules are configured in `.seedgo/bypass.json`. Supports: - **Scope:** `branch_level` - **Entry point:** `check_branch(branch_path, bypass_rules)` - **Standard label:** `TEST_QUALITY` + +--- + +## History + +- **v4.0 (2026-03-27):** Consolidated `test_coverage` into `test_quality` as category 11. Total: 51 items across 11 categories. +- **v3.0 (2026-03-24):** Expanded from 8 to 48 items across 10 categories. diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality_check.py b/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality_check.py index e126f367..284aa1f7 100644 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality_check.py +++ b/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality_check.py @@ -1,24 +1,29 @@ # =================== AIPass ==================== # Name: test_quality_check.py -# Description: Test Quality Standards Checker — 10 standard categories -# Version: 3.0.0 +# Description: Test Quality Standards Checker — 11 categories (consolidated) +# Version: 4.0.0 # Created: 2026-03-24 -# Modified: 2026-03-24 +# Modified: 2026-03-27 # ============================================= """ Test Quality Standards Checker Handler Branch-level checker that scans ALL test files in a branch's tests/ -directory and evaluates coverage across 10 standard test categories. +directory and evaluates coverage across 11 standard test categories +(10 pattern categories + module coverage). + +Consolidates the former test_coverage_check.py (import-based module +coverage analysis) into this single comprehensive test checker. Does NOT require specific filenames. Does NOT run pytest -- analyses -test files statically via text scan. +test files statically via text scan + import mapping. Scoring model: Score = (total_items_covered / total_items) * 100 """ +import re from pathlib import Path from aipass.prax import logger @@ -26,6 +31,24 @@ from aipass.seedgo.apps.handlers.json import json_handler AUDIT_SCOPE = "branch_level" +# -- Directories to skip when scanning for module coverage -------------------- +SKIP_DIRS: set[str] = { + "__pycache__", ".archive", ".mypy_cache", ".ruff_cache", + ".pytest_cache", ".venv", "venv", "node_modules", ".git", + "site-packages", "logs", "tools", ".trinity", ".aipass", + ".ai_mail.local", ".spawn", "backups", "reports", "docs", + ".sorting_unprocessed", +} + +# -- Regex patterns for module coverage (from test_coverage_check.py) --------- +RE_TEST_FUNC = re.compile(r"^\s*(?:async\s+)?def\s+(test_\w+)", re.MULTILINE) +RE_IMPORT_FROM = re.compile( + r"from\s+(?:aipass\.)?\w+\.apps\.(?:modules|handlers)[./]?([\w.]*)\s+import" +) +RE_IMPORT_DIRECT = re.compile( + r"import\s+(?:aipass\.)?\w+\.apps\.(?:modules|handlers)[./]?([\w.]*)" +) + # -- Standard test categories and their detection patterns -------------------- STANDARD_CATEGORIES: dict[str, dict[str, list[str]]] = { # Category 1: JSON Handler (8 items) @@ -127,9 +150,13 @@ STANDARD_CATEGORIES: dict[str, dict[str, list[str]]] = { }, } -TOTAL_ITEMS = sum( - len(items) for items in STANDARD_CATEGORIES.values() -) +# Pattern-based items from STANDARD_CATEGORIES +_PATTERN_ITEMS = sum(len(items) for items in STANDARD_CATEGORIES.values()) + +# Module coverage adds 3 items: test_files_exist, test_functions_exist, module_coverage +_MODULE_COVERAGE_ITEMS = 3 + +TOTAL_ITEMS = _PATTERN_ITEMS + _MODULE_COVERAGE_ITEMS # ============================================= @@ -171,6 +198,103 @@ def _read_file_safe(path: Path) -> str: return "" +def _should_skip_dir(name: str) -> bool: + """Check if a directory name should be skipped.""" + return name in SKIP_DIRS or name.startswith(".") + + +def _find_test_files_broad(branch_path: Path) -> list[Path]: + """Find all test files for module coverage analysis. + + Broader than _find_all_test_files — also finds scattered test files + outside tests/ directory. Used for import-based module mapping. + """ + test_files: list[Path] = [] + seen: set[Path] = set() + + # 1. Standard tests/ directory + tests_dir = branch_path / "tests" + if tests_dir.is_dir(): + for py_file in sorted(tests_dir.rglob("*.py")): + if py_file.name in ("__init__.py", "conftest.py"): + continue + if "__pycache__" in py_file.parts: + continue + resolved = py_file.resolve() + if resolved not in seen: + seen.add(resolved) + test_files.append(py_file) + + # 2. Scattered test_*.py or *_test.py anywhere in the branch + for py_file in sorted(branch_path.rglob("*.py")): + if any(_should_skip_dir(part) for part in py_file.relative_to(branch_path).parts): + continue + if py_file.name in ("__init__.py", "conftest.py"): + continue + if py_file.name.startswith("test_") or py_file.name.endswith("_test.py"): + resolved = py_file.resolve() + if resolved not in seen: + seen.add(resolved) + test_files.append(py_file) + + return test_files + + +def _analyze_test_file_imports(source: str) -> set[str]: + """Extract tested module names from a test file source via import patterns.""" + tested_modules: set[str] = set() + + for match in RE_IMPORT_FROM.finditer(source): + sub_path = match.group(1) + if sub_path: + tested_modules.add(sub_path.split(".")[0]) + + for match in RE_IMPORT_DIRECT.finditer(source): + sub_path = match.group(1) + if sub_path: + tested_modules.add(sub_path.split(".")[0]) + + return tested_modules + + +def _collect_testable_modules(branch_path: Path) -> set[str]: + """Collect module names from apps/modules/ and apps/handlers/. + + Returns set of module names: + - apps/modules/*.py -> file stem + - apps/handlers/*.py -> file stem + - apps/handlers/subdir/ -> directory name if it contains .py files + """ + modules: set[str] = set() + apps_dir = branch_path / "apps" + if not apps_dir.is_dir(): + return modules + + modules_dir = apps_dir / "modules" + if modules_dir.is_dir(): + for item in sorted(modules_dir.iterdir()): + if item.is_file() and item.suffix == ".py" and item.name != "__init__.py": + modules.add(item.stem) + + handlers_dir = apps_dir / "handlers" + if handlers_dir.is_dir(): + for item in sorted(handlers_dir.iterdir()): + if _should_skip_dir(item.name): + continue + if item.is_dir() and item.name != "__pycache__": + has_py = any( + f.suffix == ".py" and f.name != "__init__.py" + for f in item.iterdir() + if f.is_file() + ) + if has_py: + modules.add(item.name) + elif item.is_file() and item.suffix == ".py" and item.name != "__init__.py": + modules.add(item.stem) + + return modules + + def _find_all_test_files(branch_path: Path) -> list[Path]: """Find all test files and conftest.py in the branch's tests/ directory. @@ -233,8 +357,8 @@ def _detect_all_coverage( def check_branch(branch_path: str, bypass_rules: list | None = None) -> dict: """Run test quality analysis on a branch. - Scans all test_*.py and conftest.py files in tests/ and evaluates - coverage across 10 standard test categories. + Scans all test files and evaluates coverage across 11 categories + (10 pattern categories + module coverage). Score = total items covered / total items. Args: @@ -318,12 +442,12 @@ def check_branch(branch_path: str, bypass_rules: list | None = None) -> dict: if source: file_sources.append((tf.name, source)) - # Phase 3: Detect coverage across all categories + # Phase 3: Detect coverage across all pattern categories all_coverage = _detect_all_coverage(file_sources) total_items_covered = 0 - # Per-category summary checks + # Per-category summary checks (10 pattern categories) for category, item_coverage in all_coverage.items(): cat_total = len(item_coverage) cat_covered = sum(1 for f in item_coverage.values() if f is not None) @@ -348,12 +472,88 @@ def check_branch(branch_path: str, bypass_rules: list | None = None) -> dict: ), }) - # Score = coverage percentage + # Phase 4: Module coverage (category 11 — from test_coverage_check.py) + # Uses broader file discovery + import-based module mapping + broad_test_files = _find_test_files_broad(bp) + total_tests = 0 + tested_modules: set[str] = set() + for tf in broad_test_files: + source = _read_file_safe(tf) + if not source: + continue + total_tests += len(RE_TEST_FUNC.findall(source)) + tested_modules.update(_analyze_test_file_imports(source)) + + if total_tests == 0: + tested_modules = set() + + all_modules = _collect_testable_modules(bp) + total_modules = len(all_modules) + + # 3 module coverage items + mc_items_covered = 0 + + # Item 1: Test files exist + has_test_files = len(broad_test_files) > 0 + if has_test_files: + mc_items_covered += 1 + + # Item 2: Test functions exist + has_test_funcs = total_tests > 0 + if has_test_funcs: + mc_items_covered += 1 + + # Item 3: Module coverage >= 25% + if total_modules > 0: + covered_count = len(tested_modules & all_modules) + coverage_pct = (covered_count / total_modules) * 100 + else: + covered_count = 0 + coverage_pct = 100.0 # Nothing to test = full coverage + + has_module_coverage = coverage_pct >= 25 or total_modules == 0 + if has_module_coverage: + mc_items_covered += 1 + + total_items_covered += mc_items_covered + + # Module coverage check summary + mc_details: list[str] = [] + if not has_test_files: + mc_details.append("no test files") + if not has_test_funcs: + mc_details.append("no test functions") + if not has_module_coverage: + mc_details.append(f"module coverage {coverage_pct:.0f}% < 25%") + + if mc_items_covered == _MODULE_COVERAGE_ITEMS: + mc_msg = f"module_coverage: {mc_items_covered}/{_MODULE_COVERAGE_ITEMS} covered" + if total_modules > 0: + mc_msg += f" ({covered_count}/{total_modules} modules, {total_tests} tests)" + checks.append({ + "name": "module_coverage", + "passed": True, + "message": mc_msg, + }) + else: + checks.append({ + "name": "module_coverage", + "passed": False, + "message": ( + f"module_coverage: {mc_items_covered}/{_MODULE_COVERAGE_ITEMS} covered " + f"(missing: {', '.join(mc_details)})" + ), + }) + + # Score = total coverage percentage score = int((total_items_covered / TOTAL_ITEMS) * 100) # Overall pass at 75% overall_passed = score >= 75 + # Total categories = 10 pattern + 1 module coverage = 11 + total_categories = len(STANDARD_CATEGORIES) + 1 + # Overall summary check if overall_passed: checks.append({ @@ -361,7 +561,7 @@ def check_branch(branch_path: str, bypass_rules: list | None = None) -> dict: "passed": True, "message": ( f"{total_items_covered}/{TOTAL_ITEMS} items covered " - f"across {len(STANDARD_CATEGORIES)} categories ({score}%)" + f"across {total_categories} categories ({score}%)" ), }) else: @@ -370,7 +570,7 @@ def check_branch(branch_path: str, bypass_rules: list | None = None) -> dict: "passed": False, "message": ( f"{total_items_covered}/{TOTAL_ITEMS} items covered " - f"across {len(STANDARD_CATEGORIES)} categories ({score}%) " + f"across {total_categories} categories ({score}%) " f"-- minimum 75% required" ), }) @@ -384,14 +584,25 @@ def check_branch(branch_path: str, bypass_rules: list | None = None) -> dict: "test_files": len(test_files), "items_covered": total_items_covered, "items_total": TOTAL_ITEMS, + "module_coverage": { + "covered_modules": covered_count, + "total_modules": total_modules, + "total_tests": total_tests, + }, "category_detail": { - cat: { - "covered": sum( - 1 for f in items.values() if f is not None - ), - "total": len(items), - } - for cat, items in all_coverage.items() + **{ + cat: { + "covered": sum( + 1 for f in items.values() if f is not None + ), + "total": len(items), + } + for cat, items in all_coverage.items() + }, + "module_coverage": { + "covered": mc_items_covered, + "total": _MODULE_COVERAGE_ITEMS, + }, }, }, ) diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality_content.py b/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality_content.py index 0e6dfc50..849046be 100644 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality_content.py +++ b/src/aipass/seedgo/apps/handlers/aipass_standards/test_quality_content.py @@ -1,9 +1,9 @@ # =================== AIPass ==================== # Name: test_quality_content.py # Description: Test Quality Standards Content Handler -# Version: 3.0.0 +# Version: 4.0.0 # Created: 2026-03-24 -# Modified: 2026-03-24 +# Modified: 2026-03-27 # ============================================= """ @@ -24,22 +24,27 @@ def get_test_quality_standards() -> str: """ lines = [ "[bold cyan]CORE PRINCIPLE:[/bold cyan]", - " Every branch should have tests covering 10 standard categories.", + " Every branch should have tests covering 11 standard categories.", " The checker scans ALL test files in tests/ (including conftest.py) —", " no specific filenames required. Quality = coverage breadth.", "", + " [dim]Consolidates the former test_coverage standard into this single[/dim]", + " [dim]comprehensive test checker (module coverage is category 11).[/dim]", + "", "[bold cyan]WHAT IT CHECKS:[/bold cyan]", " Branch-level static analysis (does NOT run pytest):", "", - " [yellow]1. Test file discovery:[/yellow]", + " [yellow]1. Pattern coverage (categories 1-10):[/yellow]", " Scans [dim]tests/[/dim] for ALL [dim]test_*.py[/dim] files + [dim]conftest.py[/dim]", - " No naming requirements — any test file counts", - "", - " [yellow]2. Category coverage:[/yellow]", " For each of 10 categories, checks if test files reference", " the expected patterns. Reports per-category: X/N covered", "", - "[bold cyan]THE 10 CATEGORIES (48 items total):[/bold cyan]", + " [yellow]2. Module coverage (category 11):[/yellow]", + " Discovers test files broadly (tests/ + scattered test_*.py)", + " Maps tested modules via import patterns", + " Checks: test files exist, test functions exist, module coverage >= 25%", + "", + "[bold cyan]THE 11 CATEGORIES (51 items total):[/bold cyan]", "", " [bold]1. JSON Handler (8 items)[/bold]", " [dim]default_factory, validate, get_path, ensure_exists,[/dim]", @@ -89,13 +94,18 @@ def get_test_quality_standards() -> str: " [dim]autouse_fixtures, sys_modules_mock, reimport_after_mock[/dim]", " Template: seedgo/templates/test_conftest_template.py", "", + " [bold]11. Module Coverage (3 items)[/bold]", + " [dim]test_files_exist, test_functions_exist, module_coverage_25pct[/dim]", + " Uses import-based module mapping (from aipass..apps...)", + " Threshold: 25% of modules covered via imports", + "", "[bold cyan]SCORING MODEL:[/bold cyan]", "", - " Score = (items_covered / 48) * 100", + " Score = (items_covered / 51) * 100", "", - " Overall pass threshold: [yellow]75%[/yellow] (36+ of 48 items)", + " Overall pass threshold: [yellow]75%[/yellow] (39+ of 51 items)", "", - " [dim]No test files = 0%. All 48 items covered = 100%.[/dim]", + " [dim]No test files = 0%. All 51 items covered = 100%.[/dim]", "", "[bold cyan]EXAMPLE OUTPUT:[/bold cyan]", "", @@ -103,7 +113,8 @@ def get_test_quality_standards() -> str: " [dim]cli_routing: 7/9 covered (missing: help_word, output_capture)[/dim]", " [dim]conftest_fixtures: 4/6 covered (missing: mock_infrastructure, mock_logger)[/dim]", " [dim]error_resilience: 0/4 covered (...)[/dim]", - " [dim]Overall: 23/48 items covered across 10 categories (47%)[/dim]", + " [dim]module_coverage: 3/3 covered (5/8 modules, 42 tests)[/dim]", + " [dim]Overall: 26/51 items covered across 11 categories (50%)[/dim]", "", "[bold cyan]HOW TO COMPLY:[/bold cyan]", "", @@ -126,6 +137,11 @@ def get_test_quality_standards() -> str: "[bold cyan]BYPASS:[/bold cyan]", " Via [dim].seedgo/bypass.json[/dim] — supports standard-level and", " file-level bypass rules", + "", + "[bold cyan]HISTORY:[/bold cyan]", + " [dim]v4.0 (2026-03-27): Consolidated test_coverage into test_quality[/dim]", + " [dim] Module coverage is now category 11 (3 items). Total: 51 items.[/dim]", + " [dim]v3.0 (2026-03-24): Expanded from 8 to 48 items across 10 categories[/dim]", ] json_handler.log_operation("standard_content_queried", {"standard": "test_quality"}) diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/testing.md b/src/aipass/seedgo/apps/handlers/aipass_standards/testing.md deleted file mode 100644 index f807717a..00000000 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/testing.md +++ /dev/null @@ -1,337 +0,0 @@ -# Testing Standards -**Status:** Draft v1 - Current manual process documented -**Date:** 2025-11-12 - ---- - -## Current State: Manual Testing (Effective for Rapid Iteration) - -**Reality:** AIPass is a custom framework in active evolution. System changes weekly. Building extensive test infrastructure now = updating tests constantly instead of building features. - -**Current approach:** Manual testing with JSON/log verification works effectively for rapid iteration phase. - -**Future:** pytest framework expansion once branches stabilize (pytest is already configured and operational in some branches). - ---- - -## The 90% Build Process - -**How we build and test:** - -### 1. Planning Phase (Upfront Investment) -- Issue plan through Flow (master plan or default plan depending on scope) -- Spend time getting structure right before coding -- Define what we're building and how it fits together - -### 2. Build to 90% (AI-Led, Internal Verification) -- AI builds structure and implementation -- **Internal verification tests as we go:** - - Does the module turn on? - - Do commands work? - - Basic functionality confirmed? -- Human mostly observes, provides input if things go astray -- Focus on getting structure and pieces in place - -**Linux advantage:** Same environment (AI and human both on Linux) = outputs match = internal tests are reliable. Windows had issues with this, Linux doesn't. - -### 3. 90% Threshold (Human Review Phase) -- Human gets actively involved -- Reviews structure and implementation -- Asks questions about concerns -- Performs feature tests (what module is supposed to do) -- Identifies bugs and missing error handling - -### 4. Debug Cycle (Error Handling First, Then Fixes) - -**Critical pattern: Fix error handling BEFORE fixing bugs** - -**Example scenario:** -``` -Bug: API call not working, but console says "Success" -No error output, but logs show API never executed -Output is lying - -Process: -1. Fix error handling FIRST - make errors tell the truth -2. THEN fix the actual bug (why API isn't executing) -3. See clean pass with honest outputs -4. Move to next feature -``` - -**Why this order:** -- Can't debug effectively if errors lie -- Truth in outputs = faster debugging -- Honest error messages = system teaches itself what's wrong - -### 5. Iterate Until Acceptable -- Test different features -- Continue debug cycle (handle errors → fix bugs → verify) -- Reach acceptable standard (basic or advanced depending on module) -- Move on - ---- - -## Why Manual Testing Works Right Now - -**Advantages:** -- **Fast iteration** - No test maintenance overhead -- **Flexible** - System can change tomorrow without breaking test suite -- **JSON/log infrastructure** - Acts as verification layer - - Check config.json → see settings - - Check data.json → see state - - Check log.json → see operations history - - Check Prax logs → detailed debugging -- **Linux environment match** - AI tests = human tests (same outputs) -- **Effective debugging** - Error-first approach catches issues early - -**Tradeoffs:** -- Manual effort required at 90% stage -- No automated regression testing (yet) -- Relies on human review for final verification - -**Acceptable because:** System is evolving rapidly. Better to build fast and test manually than build slow and maintain brittle tests. - ---- - -## Future: pytest Framework - -**Infrastructure in place:** -- `pytest.ini` at repo root - Configuration for test discovery and markers -- `tests/` - Root test directory with conftest.py -- Branch-specific test directories at `src/aipass/{branch}/tests/` (API, Prax, CLI, etc.) - -**Already operational in some branches:** -- API branch: 4 test files (test_api_system.py, test_openrouter_key.py, etc.) -- Prax branch: test_log_rotation.py validates rotation behavior -- Seedgo branch: test_cli_errors.py demonstrates error handling patterns - -**When to expand testing:** -- Once modules and branches stabilize -- When system changes slow down (monthly, not weekly) -- When maintenance cost < value of automation - -**Why pytest:** -- Standard Python testing framework -- Already configured with pytest.ini -- Infrastructure exists, ready for expansion -- Some branches already have working tests - -**Current selective approach:** -- Test critical/stable components (API, Prax log rotation) -- Skip testing rapidly changing features -- Manual testing for experimental work -- Automated tests where they add value without maintenance burden - ---- - -## Error Handling Philosophy - -**Errors must tell the truth** - foundational to testing effectiveness - -**Good error handling:** -```python -try: - result = api_call() - if not result: - logger.error("API call failed - no response") - return {"success": False, "error": "API returned no data"} -except Exception as e: - logger.error(f"API call exception: {e}", exc_info=True) - return {"success": False, "error": str(e)} -``` - -**Bad error handling:** -```python -try: - result = api_call() - return {"success": True} # LIES - didn't check if result valid -except: - pass # Silent failure - no truth -``` - -**Testing relies on honest errors:** -- If errors lie, testing is impossible -- Fix error handling first = testing becomes possible -- Then fix bugs with confident verification - ---- - -## JSON/Log Infrastructure as Testing Layer - -**Three-JSON pattern supports testing:** - -### Config Verification -```bash -# Check if settings are correct -cat module_name_config.json -# See API keys, limits, feature toggles -``` - -### State Verification -```bash -# Check current state -cat module_name_data.json -# See metrics, counts, current status -``` - -### Operations Verification -```bash -# Check what actually happened -cat module_name_log.json -# See recent operations and results -``` - -### Detailed Debugging -```bash -# Prax provides file-based logging, not real-time watching -# Check system logs directory for detailed output -ls -la system_logs/ -# Read specific module logs (Prax manages this directory location) -cat system_logs/module_name.log -``` - -**This infrastructure = verification layer without formal tests** - ---- - -## Testing Workflow Example - -**Building a new branch creation module:** - -1. **Plan** - Define structure, features, workflow (Flow plan) - -2. **Build to 90%** - AI implements: - - Module structure - - Handler functions - - Config/data/log JSONs - - Internal verification: "Does `create_branch test_branch` work?" - -3. **Review at 90%** - Human tests: - - Create branch with various names - - Check if files copied correctly - - Verify registry updated - - Try edge cases (existing branch, invalid name) - -4. **Find bug** - Branch created but registry not updated - - **First:** Check error handling - is error logged? Is return value honest? - - Add error handling if missing - - **Then:** Fix bug - why isn't registry updating? - - Verify with clean pass - -5. **Iterate** - Test more features: - - Template copying - - Placeholder replacement - - Memory file handling - - Backup on conflicts - -6. **Acceptable** - All major features work, errors are honest, ready to use - ---- - -## Current Testing Checklist - -**For any new module/feature:** - -- [ ] Does it turn on without errors? -- [ ] Do basic commands work? -- [ ] Are errors handled and logged? -- [ ] Do outputs tell the truth? -- [ ] Check config.json - settings correct? -- [ ] Check data.json - state tracking working? -- [ ] Check log.json - operations recorded? -- [ ] Test edge cases (invalid input, missing files, etc.) -- [ ] Check Prax logs in system_logs/ for detailed debugging info -- [ ] Manual feature tests at 90% stage - ---- - -## When to Test What - -**During development (AI internal verification):** -- Module starts without errors -- Basic commands execute -- Expected output appears - -**At 90% stage (human testing):** -- Feature functionality (does it do what it's supposed to?) -- Edge cases (what breaks it?) -- Error handling (are errors honest?) -- Integration (does it work with other modules?) - -**Before considering "done":** -- Clean passes on major features -- Errors tell the truth (no silent failures) -- JSON logs show operations correctly -- Acceptable standard reached (basic or advanced) - ---- - -## Summary - -**Current approach:** Manual testing with JSON/log infrastructure - -**Why it works:** -- Fast iteration without test maintenance -- Linux environment = reliable verification -- Error-first debugging = effective bug fixing -- JSON/log system = verification layer - -**Build process:** Plan → Build to 90% → Review/test → Debug (errors first, then bugs) → Iterate → Acceptable - -**Future:** pytest framework when system stabilizes - -**Philosophy:** Build fast, verify as you go, handle errors honestly, iterate rapidly. Test infrastructure comes after stability. - ---- - -## Pytest Test Structure - -**Current implementation:** -``` -pytest.ini # Test configuration (repo root) -tests/ # Root test directory - conftest.py # Shared fixtures -src/aipass/ - api/tests/ # API tests (4 files operational) - test_api_system.py - test_openrouter_key.py - test_free_models_quick.py - test_paid_model.py - prax/tests/ # Prax tests - test_log_rotation.py - cli/tests/ # CLI tests (infrastructure ready) - seedgo/ - tests/conftest.py - apps/modules/test_cli_errors.py # Demo module -``` - -**Running tests:** -```bash -# Run all tests -pytest - -# Run specific branch tests -pytest src/aipass/api/tests/ - -# Run with markers -pytest -m unit -pytest -m integration -pytest -m slow -``` - -**Test demonstration module:** -- `src/aipass/seedgo/apps/modules/test_cli_errors.py` - Shows error handling patterns -- Not a pytest test, but demonstrates testing concepts -- Run directly: `python3 src/aipass/seedgo/apps/modules/test_cli_errors.py` - ---- - -## Comments - -#@comments:2025-11-13:claude: Pytest infrastructure exists and operational in API/Prax branches. Selective testing approach: automate stable components, manual testing for rapid development. - -#@comments:2025-11-13:claude: Prax doesn't have real-time "watcher" command for logs - uses file-based logging in system_logs/ (Prax manages the location). Updated documentation to reflect actual capabilities. - -#@comments:2025-11-13:claude: Error-first debugging approach is critical - maybe this should be emphasized in error_handling.md when we fill that section? - -#@comments:2025-11-13:claude: "90% threshold" is interesting pattern - not 100% perfection, but "acceptable standard" (basic or advanced). Reflects pragmatic development philosophy. diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/testing_check.py b/src/aipass/seedgo/apps/handlers/aipass_standards/testing_check.py deleted file mode 100644 index 1732047d..00000000 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/testing_check.py +++ /dev/null @@ -1,243 +0,0 @@ -# =================== AIPass ==================== -# Name: testing_check.py -# Description: Testing Standards Checker Handler -# Version: 1.0.0 -# Created: 2026-03-05 -# Modified: 2026-03-05 -# ============================================= - -""" -Testing Standards Checker Handler - -Validates testing compliance with AIPass testing standards. -Checks for test functions, error handling patterns. -Note: Manual testing is acceptable in current rapid iteration phase. -""" - -import sys -import re -from pathlib import Path -from typing import Dict, List, Optional -from aipass.prax import logger -from aipass.seedgo.apps.handlers.json import json_handler - -# Audit scope: all Python files -AUDIT_SCOPE = "all_files" - -def is_bypassed(file_path: str, standard: str, line: int | None = None, bypass_rules: list | None = None) -> bool: - """Check if a violation should be bypassed""" - if not bypass_rules: - return False - for rule in bypass_rules: - # Must match standard - if rule.get('standard') and rule.get('standard') != standard: - continue - # Must match file (check if rule file path is in the full path) - rule_file = rule.get('file', '') - if rule_file and rule_file not in file_path: - continue - # Check line-specific bypass - rule_lines = rule.get('lines', []) - if rule_lines and line is not None and line not in rule_lines: - continue - return True - return False - - -def check_module(module_path: str, bypass_rules: list | None = None) -> Dict: - """ - Check if module follows testing standards - - Args: - module_path: Path to Python module to check - bypass_rules: Optional list of bypass rules to skip certain checks - - Returns: - dict: { - 'passed': bool, # Overall pass/fail - 'checks': [ # Individual check results - { - 'name': str, # Check name - 'passed': bool, # Pass/fail - 'message': str, # Details - } - ], - 'score': int, # 0-100 percentage - 'standard': str # Standard name - } - """ - checks = [] - path = Path(module_path) - - # Check if entire standard is bypassed for this file - if is_bypassed(module_path, 'testing', bypass_rules=bypass_rules): - return { - 'passed': True, - 'checks': [{'name': 'Bypassed', 'passed': True, 'message': 'Standard bypassed via .seedgo/bypass.json'}], - 'score': 100, - 'standard': 'TESTING' - } - - # Validate file exists - if not path.exists(): - return { - 'passed': False, - 'checks': [{'name': 'File exists', 'passed': False, 'message': f'File not found: {module_path}'}], - 'score': 0, - 'standard': 'TESTING' - } - - # Read file - try: - with open(path, 'r', encoding='utf-8') as f: - content = f.read() - lines = content.split('\n') - except Exception as e: - logger.info("Cannot read %s: %s", path, e) - return { - 'passed': False, - 'checks': [{'name': 'File readable', 'passed': False, 'message': f'Error reading file: {e}'}], - 'score': 0, - 'standard': 'TESTING' - } - - # Check 1: Error handling presence (for non-test files) - is_test_file = path.name.startswith('test_') or path.name.startswith('test.') - if not is_test_file: - error_handling_check = check_error_handling(content, lines, module_path) - if error_handling_check: - checks.append(error_handling_check) - - # Check 2: Test functions (if it's a test file) - if is_test_file: - test_functions_check = check_test_functions(content) - checks.append(test_functions_check) - - # If no checks were added, testing standard doesn't apply (passes) - if not checks: - return { - 'passed': True, - 'checks': [{'name': 'Testing check', 'passed': True, 'message': 'Manual testing acceptable (no automated tests required)'}], - 'score': 100, - 'standard': 'TESTING' - } - - # Calculate score - passed_checks = sum(1 for check in checks if check['passed']) - total_checks = len(checks) - score = int((passed_checks / total_checks * 100)) if total_checks > 0 else 0 - - # Overall pass if score >= 75% - overall_passed = score >= 75 - - json_handler.log_operation("check_completed", {"file": str(module_path), "score": score, "standard": "testing"}) - return { - 'passed': overall_passed, - 'checks': checks, - 'score': score, - 'standard': 'TESTING' - } - - -def _is_silent_except(lines: List[str], pass_index: int, pass_line: str) -> bool: - pass_indent = len(pass_line) - len(pass_line.lstrip()) - for j in range(pass_index, min(pass_index + 3, len(lines))): - next_line = lines[j].strip() - is_pass_line = next_line == 'pass' or next_line.startswith('pass ') or next_line.startswith('pass#') - if next_line and not is_pass_line: - if lines[j].startswith(' ') and len(lines[j]) - len(lines[j].lstrip()) > pass_indent: - return False - break - return True - - -def check_error_handling(content: str, lines: List[str], module_path: str = "") -> Optional[Dict]: - """ - Check for proper error handling patterns - - Good: try/except with logging or return values - Bad: Silent failures (bare except: pass) - """ - # Count try/except blocks - try_count = content.count('try:') - except_count = content.count('except') - - if try_count == 0: - # No error handling, but that's acceptable (not all code needs it) - return None - - # Check for silent failures (bare except with only pass) - silent_failures = [] - in_docstring = False - in_except = False - except_line = 0 - - for i, line in enumerate(lines): - stripped = line.strip() - - # Track docstrings (skip single-line docstrings) - if stripped.startswith('"""') or stripped.startswith("'''"): - # Check if it's a single-line docstring (opens and closes on same line) - quote = '"""' if stripped.startswith('"""') else "'''" - if stripped.count(quote) == 2 and len(stripped) > len(quote) * 2: - # Single-line docstring, don't toggle - pass - else: - in_docstring = not in_docstring - - # Skip docstrings - if in_docstring: - continue - - # Track except blocks - if 'except' in stripped and ':' in stripped: - in_except = True - except_line = i - continue - - # Check if except block only has pass - if in_except: - # Check if line is just 'pass' or 'pass' with a comment - if stripped == 'pass' or stripped.startswith('pass ') or stripped.startswith('pass#'): - if _is_silent_except(lines, i, line): - silent_failures.append(f"line {except_line}") - - # Reset except tracking when we leave the block (check original line, not stripped) - if line.strip() and not line.startswith(' ') and not line.startswith('\t'): - in_except = False - - if silent_failures: - return { - 'name': 'Error handling', - 'passed': False, - 'message': f'Silent failure detected (except: pass) in {Path(module_path).name if module_path else "file"} at {silent_failures[0]} - errors should log/return' - } - - return { - 'name': 'Error handling', - 'passed': True, - 'message': f'Error handling present ({try_count} try/except blocks with proper handling)' - } - - -def check_test_functions(content: str) -> Dict: - """ - Check that test files have test functions - - Test files should have functions starting with test_ - """ - # Count test functions - test_functions = re.findall(r'def\s+(test_\w+)\s*\(', content) - - if not test_functions: - return { - 'name': 'Test functions', - 'passed': False, - 'message': 'Test file has no test functions (should have def test_* functions)' - } - - return { - 'name': 'Test functions', - 'passed': True, - 'message': f'Test file has {len(test_functions)} test functions' - } diff --git a/src/aipass/seedgo/apps/handlers/aipass_standards/testing_content.py b/src/aipass/seedgo/apps/handlers/aipass_standards/testing_content.py deleted file mode 100644 index 6e244e9d..00000000 --- a/src/aipass/seedgo/apps/handlers/aipass_standards/testing_content.py +++ /dev/null @@ -1,147 +0,0 @@ -# =================== AIPass ==================== -# Name: testing_content.py -# Description: Testing Standards Content Handler -# Version: 1.0.0 -# Created: 2026-03-09 -# Modified: 2026-03-09 -# ============================================= - -""" -Testing Standards Content Handler - -Provides formatted testing standards content. -Module orchestrates, handler implements. -""" - -from aipass.seedgo.apps.handlers.json import json_handler - - -def get_testing_standards() -> str: - """Return formatted testing standards content with Rich markup. - - Returns: - str: Formatted standards text with Rich styling - """ - lines = [ - "[bold red]TESTING STANDARDS[/bold red]", - "", - "[yellow]CURRENT STATE:[/yellow] Manual testing with JSON/log verification", - "[dim]Future: pytest framework expansion once branches stabilize[/dim]", - "", - "─" * 70, - "", - "[bold cyan]THE 90% BUILD PROCESS:[/bold cyan]", - "", - "[bold cyan]1. Planning Phase[/bold cyan]", - " [green]✓[/green] Issue plan through Flow (master or default plan)", - " [green]✓[/green] Define structure before coding", - "", - "[bold cyan]2. Build to 90% (AI-Led)[/bold cyan]", - " [green]✓[/green] AI builds structure and implementation", - " [green]✓[/green] Internal verification as you go:", - " [dim]- Does the module turn on?[/dim]", - " [dim]- Do commands work?[/dim]", - " [dim]- Basic functionality confirmed?[/dim]", - "", - "[bold cyan]3. 90% Threshold (Human Review)[/bold cyan]", - " [green]✓[/green] Human reviews structure and implementation", - " [green]✓[/green] Feature tests (does it do what it should?)", - " [green]✓[/green] Identifies bugs and missing error handling", - "", - "[bold cyan]4. Debug Cycle[/bold cyan]", - " [yellow]CRITICAL:[/yellow] Fix error handling BEFORE fixing bugs", - " [dim]1. Fix error handling - make errors tell the truth[/dim]", - " [dim]2. Then fix the actual bug[/dim]", - " [dim]3. See clean pass with honest outputs[/dim]", - "", - "[bold cyan]5. Iterate Until Acceptable[/bold cyan]", - " [green]✓[/green] Test features, debug cycle, reach acceptable standard", - "", - "─" * 70, - "", - "[bold yellow]ERROR HANDLING PHILOSOPHY:[/bold yellow]", - "", - "[yellow]RULE:[/yellow] Errors must tell the truth", - "", - "[green]✓ Good error handling:[/green]", - " [dim]try:[/dim]", - " [dim]result = api_call()[/dim]", - " [dim]if not result:[/dim]", - " [dim]logger.error('API call failed - no response')[/dim]", - " [dim]return {'success': False, 'error': 'API returned no data'}[/dim]", - " [dim]except Exception as e:[/dim]", - " [dim]logger.error(f'API call exception: {e}', exc_info=True)[/dim]", - " [dim]return {'success': False, 'error': str(e)}[/dim]", - "", - "[red]✗ Bad error handling:[/red]", - " [dim]try:[/dim]", - " [dim]result = api_call()[/dim]", - " [dim]return {'success': True} # LIES - didn't check result[/dim]", - " [dim]except:[/dim]", - " [dim]pass # Silent failure - no truth[/dim]", - "", - "─" * 70, - "", - "[bold yellow]JSON/LOG VERIFICATION LAYER:[/bold yellow]", - "", - "[bold cyan]Config Verification:[/bold cyan]", - " [dim]cat module_name_config.json # Settings, keys, toggles[/dim]", - "", - "[bold cyan]State Verification:[/bold cyan]", - " [dim]cat module_name_data.json # Metrics, counts, status[/dim]", - "", - "[bold cyan]Operations Verification:[/bold cyan]", - " [dim]cat module_name_log.json # Recent operations and results[/dim]", - "", - "[bold cyan]Detailed Debugging:[/bold cyan]", - " [dim]cat system_logs/module_name.log # Prax file-based logging[/dim]", - "", - "─" * 70, - "", - "[bold yellow]TESTING CHECKLIST:[/bold yellow]", - "", - " [green]✓[/green] Does it turn on without errors?", - " [green]✓[/green] Do basic commands work?", - " [green]✓[/green] Are errors handled and logged?", - " [green]✓[/green] Do outputs tell the truth?", - " [green]✓[/green] Check config.json - settings correct?", - " [green]✓[/green] Check data.json - state tracking working?", - " [green]✓[/green] Check log.json - operations recorded?", - " [green]✓[/green] Test edge cases (invalid input, missing files)", - " [green]✓[/green] Check Prax logs for detailed debugging", - " [green]✓[/green] Manual feature tests at 90% stage", - "", - "─" * 70, - "", - "[bold yellow]PYTEST INFRASTRUCTURE:[/bold yellow]", - "", - "[dim]pytest.ini[/dim] Root config", - "[dim]tests/conftest.py[/dim] Shared fixtures", - "[dim]src/aipass//tests/[/dim] Branch-specific tests", - "", - "[yellow]RULE:[/yellow] Expand automated tests when:", - " [dim]- Modules and branches stabilize[/dim]", - " [dim]- System changes slow down (monthly, not weekly)[/dim]", - " [dim]- Maintenance cost < value of automation[/dim]", - "", - "[yellow]RULE:[/yellow] Selective approach:", - " [green]✓[/green] Test critical/stable components (API, Prax rotation)", - " [green]✓[/green] Skip testing rapidly changing features", - " [green]✓[/green] Manual testing for experimental work", - " [green]✓[/green] Automated tests where they add value", - "", - "[dim]Run: pytest, pytest src/aipass//tests/, pytest -m unit[/dim]", - "", - "─" * 70, - "", - "[bold cyan]REFERENCE:[/bold cyan]", - " [dim]See: seedgo standards pack (testing)[/dim]", - " [dim]See: src/aipass/api/tests/ (working pytest examples)[/dim]", - " [dim]See: src/aipass/seedgo/apps/modules/test_cli_errors.py (demo module)[/dim]", - "", - "[bold]Status:[/bold] Draft v1 - Manual testing documented", - "[bold]Philosophy:[/bold] Build fast, verify as you go, handle errors honestly", - ] - - json_handler.log_operation("standard_content_queried", {"standard": "testing"}) - return "\n".join(lines)