feat(pr-review): evidence-based validation and trigger-driven exploration (#1593)

* gsd update * docs: define v1 requirements with holistic PR understanding 29 requirements across 6 categories: - Holistic PR Understanding (5) - context synthesis and passing - Validation Pipeline (3) - finding-validator for all reviews - Schema Enforcement (5) - VerificationEvidence required - Prompt Improvements (6) - understand intent, evidence requirements - Code Simplification (6) - remove programmatic filters - Measurement (4) - 5 PRs to validate Key addition: Pass gathered context (related files, import graph) to specialists. Currently gathered but unused. * feat(01-01): add Phase 0 synthesis instruction to orchestrator prompt - Add 'Phase 0: Understand the PR Holistically' section before Phase 1 - Include PR UNDERSTANDING output format (intent, critical changes, risk areas, files to verify) - Add explicit gate: 'Only AFTER completing Phase 0, proceed to Phase 1' - Add 'Understand First' principle to Key Principles section Covers: CONTEXT-01, CONTEXT-05 * feat(01-01): add related files and import graph to orchestrator prompt - Add related files section categorizing tests vs dependencies/callers - Add import graph section showing what files import/are imported by changed files - Limit to 30 related files (15 tests, 15 deps) and 20 import entries - Include actionable guidance for using the context Covers: CONTEXT-02, CONTEXT-03 * feat(01-02): add investigation context to specialist agent descriptions - security-reviewer: check related files for affected callers, verify tests - quality-reviewer: check related files for pattern consistency - logic-reviewer: check callers/dependents for broken assumptions - codebase-fit-reviewer: use related files to understand existing patterns - finding-validator: check related files for missed mitigations - ai-triage-reviewer: unchanged (doesn't need related file guidance) CONTEXT-04: Specialists now know which files to investigate beyond the diff * feat(01-02): add specialist-specific delegation guidance to related files section - Updated header: "Pass relevant files to specialists when delegating" - Added per-specialist guidance for security, logic, quality, codebase-fit - Added example delegation showing how to include related files in task Orchestrator now knows HOW to pass investigation context to each specialist type * feat(02-01): add VerificationEvidence class and update finding models - Add VerificationEvidence class with required code_examined, line_range_examined, verification_method fields - Add required verification field to BaseFinding - Add required verification field to ParallelOrchestratorFinding - Add is_impact_finding boolean field to ParallelOrchestratorFinding (default False) - Add checked_for_handling_elsewhere boolean field to ParallelOrchestratorFinding (default False) - Mark old evidence field as DEPRECATED in both BaseFinding and ParallelOrchestratorFinding Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test(02-01): add tests for schema enforcement and verification evidence - Add TestVerificationEvidence class with 5 tests for VerificationEvidence model - Add TestParallelOrchestratorFindingVerification class with 6 tests for verification requirement - Add TestVerificationSchemaGeneration class with 2 tests for JSON schema generation - Update existing TestSecurityFinding and TestDeepAnalysisFinding to include verification field - Import VerificationEvidence, ParallelOrchestratorFinding, BaseFinding in test imports Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(03-02): add 'What the Diff Is For' section to orchestrator - Reframe diff as question to investigate, not document to nitpick - Add 3 questions to answer before delegation - Include 'Delegate with Context' guidance - Position after Phase 0, before Phase 1 * feat(03-01): add Understand Intent phase to all specialist prompts - Add Phase 1: Understand the PR Intent to security, logic, quality, codebase_fit agents - Force AI to understand PR purpose before searching for issues - Prevents flagging intentional design decisions as bugs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(03-02): enhance delegation guidance with context requirements - Add Context-Rich Delegation section with 3 requirements - Include PR intent summary, specific concerns, files of interest - Show anti-pattern vs good pattern comparison - Update example delegation with specific verification items * feat(03-01): add Evidence Requirements and Valid Outputs sections - Add Evidence Requirements section documenting VerificationEvidence schema - Document code_examined, line_range_examined, verification_method fields - Document is_impact_finding and checked_for_handling_elsewhere fields - Add Valid Outputs section allowing no-issues as valid output - Document invalid outputs (forced issues, theoretical edge cases) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(03-01): update output format examples with verification object - Add verification object with code_examined, line_range_examined, verification_method - Add is_impact_finding and checked_for_handling_elsewhere fields - Use domain-appropriate verification_method values per agent - Security: direct_code_inspection for injection examples - Logic: direct_code_inspection for off-by-one and race conditions - Quality: direct_code_inspection + cross_file_trace for duplication - Codebase fit: cross_file_trace for reinvention, direct_code_inspection for naming Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(04-01): add _verify_line_numbers() method - Pre-filter findings with invalid line numbers before AI validation - Cache file line counts to avoid re-reading same file - Reject findings where line > file length - Log each rejection with finding ID and reason - Conservative: allow findings if file read fails * feat(04-02): add hypothesis-validation structure to finding validator - Add "Hypothesis-Validation Structure (MANDATORY)" section with 4 steps - Define TRUE/FALSE conditions for hypothesis testing - Include worked example showing confirmed_valid conclusion path - Include counter-example showing dismissed_false_positive path - Reference structure from Investigation Process section * feat(04-01): add _validate_findings() method - Import FindingValidationResponse from pydantic_models - Create finding-validator agent client with pr_finding_validator type - Build validation prompt with findings JSON and changed files - Filter findings by validation_status: - confirmed_valid: keep with validation evidence - dismissed_false_positive: exclude from results - needs_human_review: keep with [NEEDS REVIEW] prefix - Fail-safe: return original findings on any error - Log validation statistics * feat(04-01): wire validation pipeline into review() method - Stage 1: Line verification after cross-validation (cheap pre-filter) - Stage 2: AI validation for findings that pass line check - Update programmatic filter loop to use validated_by_ai - Log validation statistics at each stage - Uses project_root (worktree or fallback) for file access * refactor(05-01): remove evidence filter and confidence routing from review() - Remove _validate_finding_evidence() call from loop - Remove _apply_confidence_routing() call - Simplify loop to only check scope - Replace routed_findings with direct validated_findings assignment * feat(05-02): remove false positive patterns from validator - Remove VAGUE_PATTERNS constant (10 patterns) - Remove GENERIC_PATTERNS constant (6 patterns) - Remove _is_false_positive() method (44 lines) - Remove _is_false_positive call from _is_valid() - Remove TestFalsePositiveDetection class (4 tests) - Update test_low_severity_higher_threshold to use actionability score REMOVE-04: VAGUE_PATTERNS, GENERIC_PATTERNS deleted REMOVE-05: _is_false_positive() method deleted * refactor(05-01): remove redundant functions and simplify scope check - Remove ConfidenceTier enum (no longer used) - Remove _validate_finding_evidence function (schema enforces evidence) - Remove _apply_confidence_routing method (validation is binary) - Remove 'from enum import Enum' import - Simplify _is_finding_in_scope to use schema field is_impact_finding instead of keyword detection * fix(pr-review): add Task tool to orchestrator configs for SDK subagents The pr_orchestrator_parallel and pr_followup_parallel agents need the Task tool in their tools list to invoke SDK subagents (security-reviewer, logic-reviewer, etc.). Without Task, the SDK cannot spawn subagents, resulting in "Agent type not found" errors. Also fixes test_integration_phase4.py to set is_impact_finding as an attribute rather than constructor arg, since PRReviewFinding doesn't have this field (it's on ParallelOrchestratorFinding Pydantic model). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(pr-review): add explicit Task tool invocation syntax for specialist agents The orchestrator was using the built-in general-purpose agent instead of our custom specialist agents (security-reviewer, logic-reviewer, etc.) because the prompt described agents but didn't show explicit Task tool invocation syntax. Changes: - Add "CRITICAL: How to Invoke Specialist Agents" section with exact subagent_type values in a reference table - Add Task tool invocation format with example syntax - Add example showing parallel invocation of multiple specialists - Add explicit "DO NOT USE" section warning against general-purpose - Update example delegation to use Task tool syntax instead of prose - Add example validation invocation for finding-validator This ensures Claude uses our custom specialists instead of defaulting to the built-in general-purpose agent. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(pr-review): implement evidence-based validation and trigger-driven exploration Major enhancements to the PR review system: **Evidence-Based Validation:** - Shift from confidence-based to evidence-based finding validation - All findings now require VerificationEvidence with code_examined, line_range_examined - finding-validator validates ALL findings (CRITICAL through LOW) before output - Add dismissed_findings array for transparency - users see what was investigated **Trigger-Driven Exploration (6 Semantic Triggers):** - OUTPUT CONTRACT CHANGED - function returns different value/type/structure - INPUT CONTRACT CHANGED - parameters added/removed/reordered - BEHAVIORAL CONTRACT CHANGED - same I/O but different internal behavior - SIDE EFFECT CONTRACT CHANGED - observable effects added/removed - FAILURE CONTRACT CHANGED - error handling changed - NULL/UNDEFINED CONTRACT CHANGED - null handling changed Orchestrator detects triggers in Phase 1 and passes them to specialists with explicit "TRIGGER:", "EXPLORATION REQUIRED:", "Stop when:" instructions. **Implementation Changes:** - Add _PRDebugLogger for comprehensive agent communication logging - Add CI status integration to verdict logic (failing CI blocks merge) - Extract with_working_dir() to shared agent_utils.py module - Inject working directory into all subagent prompts - Bump SDK requirement to >=0.1.22 for custom subagent support **Frontend:** - Tighten AUTH_FAILURE_PATTERNS to avoid false positives on AI auth discussion - Update tests for new pattern requirements Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(pr-review): wait for both queued AND in_progress CI checks Previously, the CI wait logic only blocked on "in_progress" checks, but not "queued" checks. This meant if a CI check (like CodeRabbit) was queued but not yet running, the review would start immediately and report "CI is pending" - which would be stale by the time the contributor sees it. Now we wait for ALL checks to reach "completed" status before starting the review, ensuring the CI status in our review is accurate. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: remove duplicate .planning entries from .gitignore Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: remove docs/ from git tracking (already in .gitignore) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(pr-review): propagate is_impact_finding field to allow impact findings The is_impact_finding field was defined in ParallelOrchestratorFinding but never propagated to PRReviewFinding, causing ALL impact findings (findings about callers/affected files outside the PR's changed files) to be incorrectly filtered out as "not in scope". Changes: - Add is_impact_finding field to PRReviewFinding dataclass - Extract and pass is_impact_finding in _create_finding_from_structured() - Add to to_dict() and from_dict() for serialization This enables the trigger-driven exploration feature to actually work, allowing the review to report issues in files affected by contract changes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(pr-review): add Task tool invocation syntax to followup orchestrator The follow-up review orchestrator was missing explicit Task tool invocation syntax and examples. The AI didn't know HOW to invoke the specialist agents (resolution-verifier, finding-validator, etc.), causing resolution checking to never happen. Added: - Exact agent names table (subagent_type values) - Task tool invocation format with examples - Complete follow-up review workflow with Task calls - DO NOT USE section (avoid general-purpose, Explore, Plan) - Decision matrix for when to invoke each agent - Explicit Task tool calls in Phase 2 workflow This matches the main orchestrator prompt which has extensive Task tool examples and works correctly. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(pr-review): propagate is_impact_finding in follow-up reviewer Applied the same is_impact_finding propagation fix to the follow-up reviewer that was already applied to the main orchestrator reviewer. Fixes: 1. Add is_impact_finding field to ParallelFollowupFinding Pydantic model 2. Propagate is_impact_finding when creating PRReviewFinding for new findings 3. Copy is_impact_finding from original finding for unresolved findings Without this fix, impact findings (about callers/affected files outside the PR's changed files) would be incorrectly filtered as "not in scope" during follow-up reviews. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(auth): enhance keychain service integration with config directory support Added functionality to support profile-specific credentials by introducing a hash-based service name for macOS Keychain and updating Windows credential retrieval to utilize a provided config directory. This ensures that tokens are fetched from the correct profile-specific storage locations, improving credential management across different environments. Changes include: - New functions for calculating config directory hashes and generating keychain service names. - Updated `get_token_from_keychain` and related functions to accept an optional config directory argument. - Enhanced logging for better debugging when no token is found. This aligns the backend credential handling with the frontend's expectations for profile-specific storage. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-29 14:34:08 +01:00
parent eee97e7ea5
commit bfc232825b
28 changed files with 3478 additions and 1081 deletions
@@ -173,4 +173,4 @@ OPUS_ANALYSIS_AND_IDEAS.md
 .security-key
 /shared_docs
 logs/security/
-Agents.md
+Agents.md
@@ -270,18 +270,30 @@ AGENT_CONFIGS = {
        "thinking_default": "high",
    },
    "pr_orchestrator_parallel": {
-        "tools": BASE_READ_TOOLS + WEB_TOOLS,  # Read-only for parallel PR orchestrator
+        # Read-only for parallel PR orchestrator
+        # NOTE: Do NOT add "Task" here - the SDK auto-allows Task when agents are defined
+        # via the --agents flag. Explicitly adding it interferes with agent registration.
+        "tools": BASE_READ_TOOLS + WEB_TOOLS,
        "mcp_servers": ["context7"],
        "auto_claude_tools": [],
        "thinking_default": "high",
    },
    "pr_followup_parallel": {
-        "tools": BASE_READ_TOOLS
-        + WEB_TOOLS,  # Read-only for parallel followup reviewer
+        # Read-only for parallel followup reviewer
+        # NOTE: Do NOT add "Task" here - same reason as pr_orchestrator_parallel
+        "tools": BASE_READ_TOOLS + WEB_TOOLS,
        "mcp_servers": ["context7"],
        "auto_claude_tools": [],
        "thinking_default": "high",
    },
+    "pr_finding_validator": {
+        # Standalone validator for re-checking findings against actual code
+        # Called separately from orchestrator to validate findings with fresh context
+        "tools": BASE_READ_TOOLS,
+        "mcp_servers": [],
+        "auto_claude_tools": [],
+        "thinking_default": "medium",
+    },
    # ═══════════════════════════════════════════════════════════════════════
    # ANALYSIS PHASES
    # ═══════════════════════════════════════════════════════════════════════
@@ -6,6 +6,7 @@ for multiple environment variables, and SDK environment variable passthrough
 for custom API endpoints.
 """

+import hashlib
 import json
 import logging
 import os
@@ -65,6 +66,48 @@ SDK_ENV_VARS = [
 ]


+def _calculate_config_dir_hash(config_dir: str) -> str:
+    """
+    Calculate hash of config directory path for Keychain service name.
+
+    This MUST match the frontend's calculateConfigDirHash() in credential-utils.ts.
+    The frontend uses SHA256 hash of the config dir path, taking first 8 hex chars.
+
+    Args:
+        config_dir: Path to the config directory (should be absolute/expanded)
+
+    Returns:
+        8-character hex hash string (e.g., "d74c9506")
+    """
+    return hashlib.sha256(config_dir.encode()).hexdigest()[:8]
+
+
+def _get_keychain_service_name(config_dir: str | None = None) -> str:
+    """
+    Get the Keychain service name for credential storage.
+
+    This MUST match the frontend's getKeychainServiceName() in credential-utils.ts.
+    All profiles use hash-based keychain entries for isolation:
+    - Profile with configDir: "Claude Code-credentials-{hash}"
+    - No configDir (legacy/default): "Claude Code-credentials"
+
+    Args:
+        config_dir: Optional CLAUDE_CONFIG_DIR path. If provided, uses hash-based name.
+
+    Returns:
+        Keychain service name (e.g., "Claude Code-credentials-d74c9506")
+    """
+    if not config_dir:
+        return "Claude Code-credentials"
+
+    # Expand ~ to home directory (matching frontend normalization)
+    expanded_dir = os.path.expanduser(config_dir)
+
+    # Calculate hash and return hash-based service name
+    hash_suffix = _calculate_config_dir_hash(expanded_dir)
+    return f"Claude Code-credentials-{hash_suffix}"
+
+
 def is_encrypted_token(token: str | None) -> bool:
    """
    Check if a token is encrypted (has "enc:" prefix).
@@ -346,36 +389,50 @@ def _try_decrypt_token(token: str | None) -> str | None:
    return token


-def get_token_from_keychain() -> str | None:
+def get_token_from_keychain(config_dir: str | None = None) -> str | None:
    """
    Get authentication token from system credential store.

    Reads Claude Code credentials from:
-    - macOS: Keychain
+    - macOS: Keychain (uses hash-based service name if config_dir provided)
    - Windows: Credential Manager
    - Linux: Secret Service API (via dbus/secretstorage)

+    Args:
+        config_dir: Optional CLAUDE_CONFIG_DIR path for profile-specific credentials.
+                   When provided, reads from hash-based keychain entry matching
+                   the frontend's storage location.
+
    Returns:
        Token string if found, None otherwise
    """
    if is_macos():
-        return _get_token_from_macos_keychain()
+        return _get_token_from_macos_keychain(config_dir)
    elif is_windows():
-        return _get_token_from_windows_credential_files()
+        return _get_token_from_windows_credential_files(config_dir)
    else:
        # Linux: use secret-service API via DBus
-        return _get_token_from_linux_secret_service()
+        return _get_token_from_linux_secret_service(config_dir)


-def _get_token_from_macos_keychain() -> str | None:
-    """Get token from macOS Keychain."""
+def _get_token_from_macos_keychain(config_dir: str | None = None) -> str | None:
+    """Get token from macOS Keychain.
+
+    Args:
+        config_dir: Optional CLAUDE_CONFIG_DIR path. When provided, uses hash-based
+                   service name (e.g., "Claude Code-credentials-d74c9506") matching
+                   the frontend's credential storage location.
+    """
+    # Get the correct service name (hash-based if config_dir provided)
+    service_name = _get_keychain_service_name(config_dir)
+
    try:
        result = subprocess.run(
            [
                "/usr/bin/security",
                "find-generic-password",
                "-s",
-                "Claude Code-credentials",
+                service_name,
                "-w",
            ],
            capture_output=True,
@@ -384,6 +441,14 @@ def _get_token_from_macos_keychain() -> str | None:
        )

        if result.returncode != 0:
+            # If hash-based lookup fails and we have a config_dir, DON'T fall back
+            # to default service name - that would return the wrong profile's token.
+            # The config_dir was provided explicitly, so we should only use that.
+            if config_dir:
+                logger.debug(
+                    f"No keychain entry found for service '{service_name}' "
+                    f"(config_dir: {config_dir})"
+                )
            return None

        credentials_json = result.stdout.strip()
@@ -397,22 +462,51 @@ def _get_token_from_macos_keychain() -> str | None:
            return None

        # Validate token format (Claude OAuth tokens start with sk-ant-oat01-)
-        if not token.startswith("sk-ant-oat01-"):
+        # Also accept encrypted tokens (enc:) which will be decrypted later
+        if not (token.startswith("sk-ant-oat01-") or token.startswith("enc:")):
            return None

+        logger.debug(f"Found token in keychain service '{service_name}'")
        return token

    except (subprocess.TimeoutExpired, json.JSONDecodeError, KeyError, Exception):
        return None


-def _get_token_from_windows_credential_files() -> str | None:
+def _get_token_from_windows_credential_files(
+    config_dir: str | None = None,
+) -> str | None:
    """Get token from Windows credential files.

    Claude Code on Windows stores credentials in ~/.claude/.credentials.json
+    For custom profiles, uses the config_dir's .credentials.json file.
+
+    Args:
+        config_dir: Optional CLAUDE_CONFIG_DIR path for profile-specific credentials.
    """
    try:
-        # Claude Code stores credentials in ~/.claude/.credentials.json
+        # If config_dir is provided, read from that directory first
+        if config_dir:
+            expanded_dir = os.path.expanduser(config_dir)
+            profile_cred_paths = [
+                os.path.join(expanded_dir, ".credentials.json"),
+                os.path.join(expanded_dir, "credentials.json"),
+            ]
+            for cred_path in profile_cred_paths:
+                if os.path.exists(cred_path):
+                    with open(cred_path, encoding="utf-8") as f:
+                        data = json.load(f)
+                        token = data.get("claudeAiOauth", {}).get("accessToken")
+                        if token and (
+                            token.startswith("sk-ant-oat01-")
+                            or token.startswith("enc:")
+                        ):
+                            logger.debug(f"Found token in {cred_path}")
+                            return token
+            # If config_dir provided but no token found, don't fall back to default
+            return None
+
+        # Default Claude Code credential paths (no profile specified)
        cred_paths = [
            os.path.expandvars(r"%USERPROFILE%\.claude\.credentials.json"),
            os.path.expandvars(r"%USERPROFILE%\.claude\credentials.json"),
@@ -425,7 +519,9 @@ def _get_token_from_windows_credential_files() -> str | None:
                with open(cred_path, encoding="utf-8") as f:
                    data = json.load(f)
                    token = data.get("claudeAiOauth", {}).get("accessToken")
-                    if token and token.startswith("sk-ant-oat01-"):
+                    if token and (
+                        token.startswith("sk-ant-oat01-") or token.startswith("enc:")
+                    ):
                        return token

        return None
@@ -434,7 +530,7 @@ def _get_token_from_windows_credential_files() -> str | None:
        return None


-def _get_token_from_linux_secret_service() -> str | None:
+def _get_token_from_linux_secret_service(config_dir: str | None = None) -> str | None:
    """Get token from Linux Secret Service API via DBus.

    Claude Code on Linux stores credentials in the Secret Service API
@@ -442,9 +538,12 @@ def _get_token_from_linux_secret_service() -> str | None:
    uses the secretstorage library which communicates via DBus.

    The credential is stored with:
-    - Label: "Claude Code-credentials"
+    - Label: "Claude Code-credentials" or "Claude Code-credentials-{hash}" for profiles
    - Attributes: {application: "claude-code"}

+    Args:
+        config_dir: Optional CLAUDE_CONFIG_DIR path for profile-specific credentials.
+
    Returns:
        Token string if found, None otherwise
    """
@@ -452,6 +551,9 @@ def _get_token_from_linux_secret_service() -> str | None:
        # secretstorage not installed, fall back to env var
        return None

+    # Get the correct service name (hash-based if config_dir provided)
+    target_label = _get_keychain_service_name(config_dir)
+
    try:
        # Get the default collection (typically "login" keyring)
        # secretstorage handles DBus communication internally
@@ -476,10 +578,10 @@ def _get_token_from_linux_secret_service() -> str | None:
        items = collection.search_items({"application": "claude-code"})

        for item in items:
-            # Check if this is the Claude Code credentials item
+            # Check if this is the correct Claude Code credentials item
            label = item.get_label()
-            # Use exact match for "Claude Code-credentials" to avoid false positives
-            if label == "Claude Code-credentials":
+            # Use exact match for target label (profile-specific or default)
+            if label == target_label:
                # Get the secret (stored as JSON string)
                secret = item.get_secret()
                if not secret:
@@ -492,11 +594,23 @@ def _get_token_from_linux_secret_service() -> str | None:
                    data = json.loads(secret)
                    token = data.get("claudeAiOauth", {}).get("accessToken")

-                    if token and token.startswith("sk-ant-oat01-"):
+                    if token and (
+                        token.startswith("sk-ant-oat01-") or token.startswith("enc:")
+                    ):
+                        logger.debug(
+                            f"Found token in secret service with label '{target_label}'"
+                        )
                        return token
                except json.JSONDecodeError:
                    continue

+        # If config_dir was provided but no token found, don't fall back
+        if config_dir:
+            logger.debug(
+                f"No secret service entry found with label '{target_label}' "
+                f"(config_dir: {config_dir})"
+            )
+
        return None

    except (
@@ -589,13 +703,37 @@ def get_auth_token(config_dir: str | None = None) -> str | None:
    env_config_dir = os.environ.get("CLAUDE_CONFIG_DIR")
    effective_config_dir = config_dir or env_config_dir

-    # If a custom config directory is specified, read from there
+    # Debug: Log which config_dir is being used for credential resolution
+    debug = os.environ.get("DEBUG", "").lower() in ("true", "1")
+    if debug and effective_config_dir:
+        service_name = _get_keychain_service_name(effective_config_dir)
+        logger.info(
+            f"[Auth] Resolving credentials for profile config_dir: {effective_config_dir} "
+            f"(Keychain service: {service_name})"
+        )
+
+    # If a custom config directory is specified, read from there first
    if effective_config_dir:
+        # Try reading from .credentials.json file in the config directory
        token = _get_token_from_config_dir(effective_config_dir)
        if token:
            return _try_decrypt_token(token)

-    # Fallback to system credential store (default locations)
+        # Also try the system credential store with hash-based service name
+        # This is needed because macOS stores credentials in Keychain, not files
+        token = get_token_from_keychain(effective_config_dir)
+        if token:
+            return _try_decrypt_token(token)
+
+        # If config_dir was explicitly provided, DON'T fall back to default keychain
+        # - that would return the wrong profile's token
+        logger.debug(
+            f"No credentials found for config_dir '{effective_config_dir}' "
+            "in file or keychain"
+        )
+        return None
+
+    # No config_dir specified - use default system credential store
    return _try_decrypt_token(get_token_from_keychain())


@@ -616,10 +754,20 @@ def get_auth_token_source(config_dir: str | None = None) -> str | None:
    # Check if token came from custom config directory (profile's configDir)
    env_config_dir = os.environ.get("CLAUDE_CONFIG_DIR")
    effective_config_dir = config_dir or env_config_dir
-    if effective_config_dir and _get_token_from_config_dir(effective_config_dir):
-        return "CLAUDE_CONFIG_DIR"
+    if effective_config_dir:
+        # Check file-based storage
+        if _get_token_from_config_dir(effective_config_dir):
+            return "CLAUDE_CONFIG_DIR"
+        # Check hash-based keychain entry for this profile
+        if get_token_from_keychain(effective_config_dir):
+            if is_macos():
+                return "macOS Keychain (profile)"
+            elif is_windows():
+                return "Windows Credential Files (profile)"
+            else:
+                return "Linux Secret Service (profile)"

-    # Check if token came from system credential store
+    # Check if token came from default system credential store
    if get_token_from_keychain():
        if is_macos():
            return "macOS Keychain"
@@ -0,0 +1,192 @@
+# PR Review System Quality Control Prompt
+
+You are a senior software architect tasked with quality-controlling an AI-powered PR review system. Your goal is to analyze the system holistically, identify gaps between intent and implementation, and provide actionable feedback.
+
+## System Overview
+
+This is a **parallel orchestrator PR review system** that:
+1. An orchestrator AI analyzes a PR and delegates to specialist agents
+2. Specialist agents (security, quality, logic, codebase-fit) perform deep reviews
+3. A finding-validator agent validates all findings against actual code
+4. The orchestrator synthesizes results into a final verdict
+
+**Key Design Principles (from vision document):**
+- Evidence-based validation (NOT confidence-based)
+- Pattern-triggered mandatory exploration (6 semantic triggers)
+- Understand intent BEFORE looking for issues
+- The diff is the question, not the answer
+
+---
+
+## FILES TO EXAMINE
+
+### Vision & Architecture
+- `docs/PR_REVIEW_99_TRUST.md` - The vision document defining 99% trust goal
+
+### Orchestrator Prompts
+- `apps/backend/prompts/github/pr_parallel_orchestrator.md` - Main orchestrator prompt
+- `apps/backend/prompts/github/pr_followup_orchestrator.md` - Follow-up review orchestrator
+
+### Specialist Agent Prompts
+- `apps/backend/prompts/github/pr_security_agent.md` - Security review agent
+- `apps/backend/prompts/github/pr_quality_agent.md` - Code quality agent
+- `apps/backend/prompts/github/pr_logic_agent.md` - Logic/correctness agent
+- `apps/backend/prompts/github/pr_codebase_fit_agent.md` - Codebase fit agent
+- `apps/backend/prompts/github/pr_finding_validator.md` - Finding validator agent
+
+### Implementation Code
+- `apps/backend/runners/github/services/parallel_orchestrator_reviewer.py` - Orchestrator implementation
+- `apps/backend/runners/github/services/parallel_followup_reviewer.py` - Follow-up implementation
+- `apps/backend/runners/github/services/pydantic_models.py` - Schema definitions (VerificationEvidence, etc.)
+- `apps/backend/runners/github/services/sdk_utils.py` - SDK utilities for running agents
+- `apps/backend/runners/github/services/review_tools.py` - Tools available to review agents
+- `apps/backend/runners/github/context_gatherer.py` - Gathers PR context (files, callers, dependents)
+
+### Models & Configuration
+- `apps/backend/runners/github/models.py` - Data models
+- `apps/backend/agents/tools_pkg/models.py` - Tool models
+
+---
+
+## ANALYSIS TASKS
+
+### 1. Vision Alignment Check
+Compare the implementation against `PR_REVIEW_99_TRUST.md`:
+
+- [ ] **Evidence-based validation**: Is the system truly evidence-based or does it still use confidence scores anywhere?
+- [ ] **6 Mandatory Triggers**: Are all 6 semantic triggers properly defined and enforced?
+  1. Output contract changed
+  2. Input contract changed
+  3. Behavioral contract changed
+  4. Side effect contract changed
+  5. Failure contract changed
+  6. Null/undefined contract changed
+- [ ] **Phase 0 (Understand Intent)**: Is it mandatory? Is it enforced before delegation?
+- [ ] **Phase 1 (Trigger Detection)**: Is it mandatory? Does it output explicit trigger analysis?
+- [ ] **Bounded Exploration**: Is exploration limited to depth 1 (direct callers only)?
+
+### 2. Prompt Quality Analysis
+For each agent prompt, check:
+
+- [ ] Does it explain WHAT to look for?
+- [ ] Does it explain HOW to verify findings?
+- [ ] Does it require evidence (code snippets, line numbers)?
+- [ ] Does it define when to STOP exploring?
+- [ ] Does it distinguish between "in scope" and "out of scope"?
+- [ ] Does it handle the "no issues found" case properly?
+
+### 3. Schema Enforcement
+Check `pydantic_models.py`:
+
+- [ ] Is `VerificationEvidence` required (not optional) on all finding types?
+- [ ] Does `VerificationEvidence` require:
+  - `code_examined` (actual code, not description)
+  - `line_range_examined` (specific lines)
+  - `verification_method` (how it was verified)
+- [ ] Are there any finding types that bypass evidence requirements?
+
+### 4. Information Flow
+Trace how information flows:
+
+- [ ] PR Context → Orchestrator: What context is provided?
+- [ ] Orchestrator → Specialists: Are triggers passed? Are known callers passed?
+- [ ] Specialists → Validator: Are all findings validated?
+- [ ] Validator → Final Output: Are false positives properly dismissed?
+
+### 5. False Positive Prevention
+Check mechanisms to prevent false positives:
+
+- [ ] Do specialists verify issues exist before reporting?
+- [ ] Does the validator re-read the actual code?
+- [ ] Are "missing X" claims (missing error handling, etc.) verified?
+- [ ] Are dismissed findings tracked for transparency?
+
+### 6. Log Analysis (ATTACH LOGS BELOW)
+When reviewing logs, check:
+
+- [ ] Did the orchestrator output PR UNDERSTANDING before delegating?
+- [ ] Did the orchestrator output TRIGGER DETECTION before delegating?
+- [ ] Were triggers passed to specialists in delegation prompts?
+- [ ] Did specialists actually explore when triggers were present?
+- [ ] Were findings validated with real code evidence?
+- [ ] Were any false positives caught by the validator?
+
+---
+
+## SPECIFIC QUESTIONS TO ANSWER
+
+1. **Trigger System Effectiveness**: Did the trigger detection system correctly identify semantic contract changes? Were there any missed triggers or false triggers?
+
+2. **Exploration Quality**: When exploration was mandated by a trigger, did specialists explore effectively? Did they stop at the right time?
+
+3. **Evidence Quality**: Are the `code_examined` fields in findings actual code snippets or just descriptions? Are line numbers accurate?
+
+4. **False Positive Rate**: How many findings were dismissed as false positives? What caused them?
+
+5. **Missing Issues**: Based on your understanding of the PR, were there any issues that SHOULD have been caught but weren't?
+
+6. **Prompt Gaps**: Are there any scenarios not covered by the current prompts?
+
+7. **Schema Gaps**: Are there any ways findings could bypass evidence requirements?
+
+---
+
+## OUTPUT FORMAT
+
+Provide your analysis in this structure:
+
+```markdown
+## Executive Summary
+[2-3 sentences on overall system health]
+
+## Vision Alignment Score: X/10
+[Brief explanation]
+
+## Critical Issues (Must Fix)
+1. [Issue]: [Description] → [Suggested Fix]
+2. ...
+
+## High Priority Improvements
+1. [Improvement]: [Why it matters] → [How to implement]
+2. ...
+
+## Medium Priority Improvements
+1. ...
+
+## Low Priority / Nice to Have
+1. ...
+
+## Log Analysis Findings
+### What Worked Well
+- ...
+
+### What Didn't Work
+- ...
+
+### Specific Recommendations from Log Analysis
+1. ...
+
+## Questions for the Team
+1. [Question that needs human input]
+2. ...
+```
+
+---
+
+## ATTACH LOGS BELOW
+
+Paste the PR review debug logs here for analysis:
+
+```
+[PASTE LOGS HERE]
+```
+
+---
+
+## IMPORTANT NOTES
+
+- Focus on **systemic issues**, not one-off bugs
+- Prioritize issues that cause **false positives** (annoying) over false negatives (missed issues)
+- Consider **language-agnostic** design - the system should work for any codebase
+- Think about **edge cases**: empty PRs, huge PRs, refactor-only PRs, CSS-only PRs
+- The goal is **99% trust** - developers should trust the review enough to act on it immediately
@@ -6,6 +6,81 @@ You are a focused codebase fit review agent. You have been spawned by the orches

 Ensure new code integrates well with the existing codebase. Check for consistency with project conventions, reuse of existing utilities, and architectural alignment. Focus ONLY on codebase fit - not security, logic correctness, or general quality.

+## Phase 1: Understand the PR Intent (BEFORE Looking for Issues)
+
+**MANDATORY** - Before searching for issues, understand what this PR is trying to accomplish.
+
+1. **Read the provided context**
+   - PR description: What does the author say this does?
+   - Changed files: What areas of code are affected?
+   - Commits: How did the PR evolve?
+
+2. **Identify the change type**
+   - Bug fix: Correcting broken behavior
+   - New feature: Adding new capability
+   - Refactor: Restructuring without behavior change
+   - Performance: Optimizing existing code
+   - Cleanup: Removing dead code or improving organization
+
+3. **State your understanding** (include in your analysis)
+   ```
+   PR INTENT: This PR [verb] [what] by [how].
+   RISK AREAS: [what could go wrong specific to this change type]
+   ```
+
+**Only AFTER completing Phase 1, proceed to looking for issues.**
+
+Why this matters: Understanding intent prevents flagging intentional design decisions as bugs.
+
+## TRIGGER-DRIVEN EXPLORATION (CHECK YOUR DELEGATION PROMPT)
+
+**FIRST**: Check if your delegation prompt contains a `TRIGGER:` instruction.
+
+- **If TRIGGER is present** → Exploration is **MANDATORY**, even if the diff looks correct
+- **If no TRIGGER** → Use your judgment to explore or not
+
+### How to Explore (Bounded)
+
+1. **Read the trigger** - What pattern did the orchestrator identify?
+2. **Form the specific question** - "Do similar functions elsewhere follow the same pattern?" (not "what's in the codebase?")
+3. **Use Grep** to find similar patterns, usages, or implementations
+4. **Use Read** to examine 3-5 relevant files
+5. **Answer the question** - Yes (report issue) or No (move on)
+6. **Stop** - Do not explore beyond the immediate question
+
+### Codebase-Fit-Specific Trigger Questions
+
+| Trigger | Codebase Fit Question to Answer |
+|---------|--------------------------------|
+| **Output contract changed** | Do other similar functions return the same type/structure? |
+| **Input contract changed** | Is this parameter change consistent with similar functions? |
+| **New pattern introduced** | Does this pattern already exist elsewhere that should be reused? |
+| **Naming changed** | Is the new naming consistent with project conventions? |
+| **Architecture changed** | Does this architectural change align with existing patterns? |
+
+### Example Exploration
+
+```
+TRIGGER: New pattern introduced (custom date formatter)
+QUESTION: Does a date formatting utility already exist?
+
+1. Grep for "formatDate\|dateFormat\|toDateString" → found utils/date.ts
+2. Read utils/date.ts → exports formatDate(date, format) with same functionality
+3. STOP - Found existing utility
+
+FINDINGS:
+- src/components/Report.tsx:45 - Implements custom date formatting
+  Existing utility: utils/date.ts exports formatDate() with same functionality
+  Suggestion: Use existing formatDate() instead of duplicating logic
+```
+
+### When NO Trigger is Given
+
+If the orchestrator doesn't specify a trigger, use your judgment:
+- Focus on pattern consistency in the changed code
+- Search for existing utilities that could be reused
+- Don't explore "just to be thorough"
+
 ## CRITICAL: PR Scope and Context

 ### What IS in scope (report these issues):
@@ -119,6 +194,92 @@ Before reporting ANY finding, you MUST:

 **Your evidence must prove the issue exists - not just that you suspect it.**

+## Evidence Requirements (MANDATORY)
+
+Every finding you report MUST include a `verification` object with ALL of these fields:
+
+### Required Fields
+
+**code_examined** (string, min 1 character)
+The **exact code snippet** you examined. Copy-paste directly from the file:
+```
+CORRECT: "cursor.execute(f'SELECT * FROM users WHERE id={user_id}')"
+WRONG:   "SQL query that uses string interpolation"
+```
+
+**line_range_examined** (array of 2 integers)
+The exact line numbers [start, end] where the issue exists:
+```
+CORRECT: [45, 47]
+WRONG:   [1, 100]  // Too broad - you didn't examine all 100 lines
+```
+
+**verification_method** (one of these exact values)
+How you verified the issue:
+- `"direct_code_inspection"` - Found the issue directly in the code at the location
+- `"cross_file_trace"` - Traced through imports/calls to confirm the issue
+- `"test_verification"` - Verified through examination of test code
+- `"dependency_analysis"` - Verified through analyzing dependencies
+
+### Conditional Fields
+
+**is_impact_finding** (boolean, default false)
+Set to `true` ONLY if this finding is about impact on OTHER files (not the changed file):
+```
+TRUE:  "This change in utils.ts breaks the caller in auth.ts"
+FALSE: "This code in utils.ts has a bug" (issue is in the changed file)
+```
+
+**checked_for_handling_elsewhere** (boolean, default false)
+For ANY claim about existing utilities or patterns:
+- Set `true` ONLY if you used Grep/Read tools to verify patterns exist/don't exist
+- Set `false` if you didn't search the codebase
+- **When true, include the search in your description:**
+  - "Searched `Grep('formatDate|dateFormat', 'src/utils/')` - found existing helper"
+  - "Searched `Grep('class.*Service', 'src/services/')` - confirmed naming pattern"
+
+```
+TRUE:  "Searched for date formatting helpers - found utils/date.ts:formatDate()"
+FALSE: "This should use an existing utility" (didn't verify one exists)
+```
+
+**If you cannot provide real evidence, you do not have a verified finding - do not report it.**
+
+**Search Before Claiming:** Never claim something "should use existing X" without first verifying X exists and fits the use case.
+
+## Valid Outputs
+
+Finding issues is NOT the goal. Accurate review is the goal.
+
+### Valid: No Significant Issues Found
+If the code is well-implemented, say so:
+```json
+{
+  "findings": [],
+  "summary": "Reviewed [files]. No codebase_fit issues found. The implementation correctly [positive observation about the code]."
+}
+```
+
+### Valid: Only Low-Severity Suggestions
+Minor improvements that don't block merge:
+```json
+{
+  "findings": [
+    {"severity": "low", "title": "Consider extracting magic number to constant", ...}
+  ],
+  "summary": "Code is sound. One minor suggestion for readability."
+}
+```
+
+### INVALID: Forced Issues
+Do NOT report issues just to have something to say:
+- Theoretical edge cases without evidence they're reachable
+- Style preferences not backed by project conventions
+- "Could be improved" without concrete problem
+- Pre-existing issues not introduced by this PR
+
+**Reporting nothing is better than reporting noise.** False positives erode trust faster than false negatives.
+
 ## Code Patterns to Flag

 ### Reinventing Existing Utilities
@@ -189,6 +350,13 @@ Provide findings in JSON format:
    "description": "This file implements custom date formatting, but the codebase already has `formatDate()` in `src/utils/date.ts` that does the same thing.",
    "category": "codebase_fit",
    "severity": "high",
+    "verification": {
+      "code_examined": "const formatted = `${date.getMonth()}/${date.getDate()}/${date.getFullYear()}`;",
+      "line_range_examined": [15, 15],
+      "verification_method": "cross_file_trace"
+    },
+    "is_impact_finding": false,
+    "checked_for_handling_elsewhere": false,
    "existing_code": "src/utils/date.ts:formatDate()",
    "suggested_fix": "Replace custom implementation with: import { formatDate } from '@/utils/date';",
    "confidence": 92
@@ -200,6 +368,13 @@ Provide findings in JSON format:
    "description": "This file uses 'customer' terminology but the rest of the codebase consistently uses 'user'. This creates confusion and makes search/navigation harder.",
    "category": "codebase_fit",
    "severity": "medium",
+    "verification": {
+      "code_examined": "export interface Customer { id: string; name: string; email: string; }",
+      "line_range_examined": [1, 5],
+      "verification_method": "direct_code_inspection"
+    },
+    "is_impact_finding": false,
+    "checked_for_handling_elsewhere": false,
    "codebase_pattern": "src/models/user.ts, src/api/users.ts, src/services/userService.ts",
    "suggested_fix": "Rename to use 'user' terminology to match codebase conventions",
    "confidence": 88
@@ -211,6 +386,13 @@ Provide findings in JSON format:
    "description": "This file is 847 lines and contains order validation, payment processing, inventory management, and notification sending. Each should be separate.",
    "category": "codebase_fit",
    "severity": "high",
+    "verification": {
+      "code_examined": "// File contains: validateOrder(), processPayment(), updateInventory(), sendNotification() - all in one file",
+      "line_range_examined": [1, 847],
+      "verification_method": "direct_code_inspection"
+    },
+    "is_impact_finding": false,
+    "checked_for_handling_elsewhere": false,
    "current_lines": 847,
    "suggested_fix": "Split into: orderValidator.ts, paymentProcessor.ts, inventoryManager.ts, notificationService.ts",
    "confidence": 95
@@ -33,6 +33,165 @@ For each finding you receive:
 4. **PROVIDE** concrete code evidence - the actual code that proves or disproves the issue
 5. **RETURN** validation status with evidence (binary decision based on what the code shows)

+## Batch Processing (Multiple Findings)
+
+You may receive multiple findings to validate at once. When processing batches:
+
+1. **Group by file** - Read each file once, validate all findings in that file together
+2. **Process systematically** - Validate each finding in order, don't skip any
+3. **Return all results** - Your response must include a validation result for EVERY finding received
+4. **Optimize reads** - If 3 findings are in the same file, read it once with enough context for all
+
+**Example batch input:**
+```
+Validate these findings:
+1. SEC-001: SQL injection at auth/login.ts:45
+2. QUAL-001: Missing error handling at auth/login.ts:78
+3. LOGIC-001: Off-by-one at utils/array.ts:23
+```
+
+**Expected output:** 3 separate validation results, one for each finding ID.
+
+## Hypothesis-Validation Structure (MANDATORY)
+
+For EACH finding you investigate, use this structured approach. This prevents rubber-stamping findings as valid without actually verifying them.
+
+### Step 1: State the Hypothesis
+
+Before reading any code, clearly state what you're testing:
+
+```
+HYPOTHESIS: The finding claims "{title}" at {file}:{line}
+
+This hypothesis is TRUE if:
+1. The code at {line} contains the specific pattern described
+2. No mitigation exists in surrounding context (+/- 20 lines)
+3. The issue is actually reachable/exploitable in this codebase
+
+This hypothesis is FALSE if:
+1. The code at {line} is different than described
+2. Mitigation exists (validation, sanitization, framework protection)
+3. The code is unreachable or purely theoretical
+```
+
+### Step 2: Gather Evidence
+
+Read the actual code. Copy-paste it into `code_evidence`.
+
+```
+FILE: {file}
+LINES: {line-20} to {line+20}
+ACTUAL CODE:
+[paste the code here - this is your proof]
+```
+
+### Step 3: Test Each Condition
+
+For each condition in your hypothesis:
+
+```
+CONDITION 1: Code contains {specific pattern from finding}
+EVIDENCE: [specific line from code_evidence that proves/disproves]
+RESULT: TRUE / FALSE / INCONCLUSIVE
+
+CONDITION 2: No mitigation in surrounding context
+EVIDENCE: [what you found or didn't find in ±20 lines]
+RESULT: TRUE / FALSE / INCONCLUSIVE
+
+CONDITION 3: Issue is reachable/exploitable
+EVIDENCE: [how input reaches this code, or why it doesn't]
+RESULT: TRUE / FALSE / INCONCLUSIVE
+```
+
+### Step 4: Conclude Based on Evidence
+
+Apply these rules strictly:
+
+| Conditions | Conclusion |
+|------------|------------|
+| ALL conditions TRUE | `confirmed_valid` |
+| ANY condition FALSE | `dismissed_false_positive` |
+| ANY condition INCONCLUSIVE, none FALSE | `needs_human_review` |
+
+**CRITICAL: Your conclusion MUST match your condition results.** If you found mitigation (Condition 2 = FALSE), you MUST conclude `dismissed_false_positive`, not `confirmed_valid`.
+
+### Worked Example
+
+```
+HYPOTHESIS: SQL injection at auth.py:45
+
+Conditions to test:
+1. User input directly in SQL string (not parameterized)
+2. No sanitization before this point
+3. Input reachable from HTTP request
+
+Evidence gathered:
+FILE: auth.py, lines 25-65
+ACTUAL CODE:
+```python
+def get_user(user_id: str) -> User:
+    # user_id comes from request.args["id"]
+    query = f"SELECT * FROM users WHERE id = {user_id}"  # Line 45
+    return db.execute(query).fetchone()
+```
+
+Testing conditions:
+CONDITION 1: User input in SQL string
+EVIDENCE: Line 45 uses f-string interpolation: f"SELECT * FROM users WHERE id = {user_id}"
+RESULT: TRUE
+
+CONDITION 2: No sanitization
+EVIDENCE: No validation between request.args["id"] (line 43) and query construction (line 45)
+RESULT: TRUE
+
+CONDITION 3: Input reachable
+EVIDENCE: Comment says "user_id comes from request.args", confirmed by caller on line 12
+RESULT: TRUE
+
+CONCLUSION: confirmed_valid (all conditions TRUE)
+CODE_EVIDENCE: "query = f\"SELECT * FROM users WHERE id = {user_id}\""
+LINE_RANGE: [45, 45]
+EXPLANATION: SQL injection confirmed - user input from request.args is interpolated directly into SQL query without parameterization or sanitization.
+```
+
+### Counter-Example: Dismissing a False Positive
+
+```
+HYPOTHESIS: XSS vulnerability at render.py:89
+
+Conditions to test:
+1. User input reaches output without encoding
+2. No sanitization in the call chain
+3. Output context allows script execution
+
+Evidence gathered:
+FILE: render.py, lines 70-110
+ACTUAL CODE:
+```python
+def render_comment(user_input: str) -> str:
+    sanitized = bleach.clean(user_input, tags=[], strip=True)  # Line 85
+    return f"<div class='comment'>{sanitized}</div>"  # Line 89
+```
+
+Testing conditions:
+CONDITION 1: User input reaches output
+EVIDENCE: Line 89 outputs user_input into HTML
+RESULT: TRUE
+
+CONDITION 2: No sanitization
+EVIDENCE: Line 85 uses bleach.clean() with tags=[] (strips ALL tags)
+RESULT: FALSE - sanitization exists
+
+CONDITION 3: Output allows scripts
+EVIDENCE: Even if injected, bleach.clean removes script tags
+RESULT: FALSE - mitigation prevents exploitation
+
+CONCLUSION: dismissed_false_positive (Condition 2 and 3 are FALSE)
+CODE_EVIDENCE: "sanitized = bleach.clean(user_input, tags=[], strip=True)"
+LINE_RANGE: [85, 89]
+EXPLANATION: The original finding missed the sanitization at line 85. bleach.clean() with tags=[] strips all HTML tags including script tags, making XSS impossible.
+```
+
 ## Investigation Process

 ### Step 1: Fetch the Code
@@ -47,6 +206,8 @@ Focus on lines around: {finding.line}

 ### Step 2: Analyze with Fresh Eyes - NEVER ASSUME

+**Follow the Hypothesis-Validation Structure above for each finding.** State your hypothesis, gather evidence, test each condition, then conclude based on the evidence. This structure prevents you from confirming findings just because they "sound plausible."
+
 **CRITICAL: Do NOT assume the original finding is correct.** The original reviewer may have:
 - Hallucinated line numbers that don't exist
 - Misread or misunderstood the code
@@ -194,6 +355,45 @@ These patterns often confirm the issue is real:
 4. **Missing error handling** in critical paths
 5. **Race conditions** with clear concurrent access

+## Cross-File Validation (For Specific Finding Types)
+
+Some findings require checking the CODEBASE, not just the flagged file:
+
+### Duplication Findings ("code is duplicated 3 times")
+
+**Before confirming a duplication finding, you MUST:**
+
+1. **Verify the duplicated code exists** - Read all locations mentioned
+2. **Check for existing helpers** - Use Grep to search for:
+   - Similar function names in `/utils/`, `/helpers/`, `/shared/`
+   - Common patterns that might already be abstracted
+   - Example: `Grep("formatDate|dateFormat|toDateString", "**/*.{ts,js}")`
+
+3. **Decide based on evidence:**
+   - If existing helper found → `dismissed_false_positive` (they should use it)
+   - Wait, no - if helper exists and they're NOT using it → `confirmed_valid` (finding is correct)
+   - If no helper exists → `confirmed_valid` (suggest creating one)
+
+**Example:**
+```
+Finding: "Duplicated YOLO mode check repeated 3 times"
+
+CROSS-FILE CHECK:
+1. Grep for "YOLO_MODE|yoloMode|bypassSecurity" in utils/ → No results
+2. Grep for existing env var pattern helpers → Found: utils/env.ts:getEnvFlag()
+3. CONCLUSION: confirmed_valid - getEnvFlag() exists but isn't being used
+   SUGGESTED_FIX: "Use existing getEnvFlag() helper from utils/env.ts"
+```
+
+### "Should Use Existing X" Findings
+
+**Before confirming, verify the existing X actually fits the use case:**
+
+1. Read the suggested existing code
+2. Check if it has the required interface/behavior
+3. If it doesn't match → `dismissed_false_positive` (can't use it)
+4. If it matches → `confirmed_valid` (should use it)
+
 ## Critical Rules

 1. **ALWAYS read the actual code** - Never rely on memory or the original finding description
@@ -204,6 +404,10 @@ These patterns often confirm the issue is real:
 6. **Look for mitigations** - Check surrounding code for sanitization/validation
 7. **Check the full context** - Read ±20 lines, not just the flagged line
 8. **Verify code exists** - Set `evidence_verified_in_file` to false if the code/line doesn't exist
+9. **SEARCH BEFORE CLAIMING ABSENCE** - If you claim something doesn't exist (no helper, no validation, no error handling), you MUST show the search you performed:
+   - Use Grep to search for the pattern
+   - Include the search command in your explanation
+   - Example: "Searched for `Grep('validateInput|sanitize', 'src/**/*.ts')` - no results found"

 ## Anti-Patterns to Avoid

@@ -45,13 +45,77 @@ Note: GitHub's API tells us IF there are conflicts but not WHICH files. The find

 ## Available Specialist Agents

-You have access to these specialist agents via the Task tool:
+You have access to these specialist agents via the Task tool.
+
+**You MUST use the Task tool with the exact `subagent_type` names listed below.** Do NOT use `general-purpose` or any other built-in agent - always use our custom specialists.
+
+### Exact Agent Names (use these in subagent_type)
+
+| Agent | subagent_type value |
+|-------|---------------------|
+| Resolution verifier | `resolution-verifier` |
+| New code reviewer | `new-code-reviewer` |
+| Comment analyzer | `comment-analyzer` |
+| Finding validator | `finding-validator` |
+
+### Task Tool Invocation Format
+
+When you invoke a specialist, use the Task tool like this:
+
+```
+Task(
+  subagent_type="resolution-verifier",
+  prompt="Verify resolution of these previous findings:\n\n1. [SEC-001] SQL injection in user.ts:45 - Check if parameterized queries now used\n2. [QUAL-002] Missing error handling in api.ts:89 - Check if try/catch was added",
+  description="Verify previous findings resolved"
+)
+```
+
+### Example: Complete Follow-up Review Workflow
+
+**Step 1: Verify previous findings are resolved**
+```
+Task(
+  subagent_type="resolution-verifier",
+  prompt="Previous findings to verify:\n\n1. [HIGH] is_impact_finding not propagated (parallel_orchestrator_reviewer.py:630)\n   - Original issue: Field not extracted from structured output\n   - Expected fix: Add is_impact_finding extraction and pass to PRReviewFinding\n\nCheck if the new commits resolve this issue. Examine the actual code.",
+  description="Verify previous findings"
+)
+```
+
+**Step 2: Validate unresolved findings (MANDATORY)**
+```
+Task(
+  subagent_type="finding-validator",
+  prompt="Validate these unresolved findings from resolution-verifier:\n\n1. [HIGH] is_impact_finding not propagated (parallel_orchestrator_reviewer.py:630)\n   - Status from resolution-verifier: unresolved\n   - Claimed issue: Field not extracted\n\nRead the ACTUAL code at line 630 and verify if this issue truly exists. Check for is_impact_finding extraction.",
+  description="Validate unresolved findings"
+)
+```
+
+**Step 3: Review new code (if substantial changes)**
+```
+Task(
+  subagent_type="new-code-reviewer",
+  prompt="Review new code in this diff for issues:\n- Security vulnerabilities\n- Logic errors\n- Edge cases not handled\n\nFocus on files: models.py, parallel_orchestrator_reviewer.py",
+  description="Review new code changes"
+)
+```
+
+### DO NOT USE
+
+- ❌ `general-purpose` - This is a generic built-in agent, NOT our specialist
+- ❌ `Explore` - This is for codebase exploration, NOT for PR review
+- ❌ `Plan` - This is for planning, NOT for PR review
+
+**Always use our specialist agents** (`resolution-verifier`, `new-code-reviewer`, `comment-analyzer`, `finding-validator`) for follow-up review tasks.
+
+---
+
+## Agent Descriptions

 ### 1. resolution-verifier
 **Use for**: Verifying whether previous findings have been addressed
 - Analyzes diffs to determine if issues are truly fixed
 - Checks for incomplete or incorrect fixes
- Provides confidence scores for each resolution
+- Provides evidence-based verification for each resolution
 - **Invoke when**: There are previous findings to verify

 ### 2. new-code-reviewer
@@ -93,34 +157,85 @@ Evaluate the follow-up context:
 - Are there previous findings to verify?
 - Are there new comments to process?

-### Phase 2: Delegate to Agents
-Based on your analysis, invoke the appropriate agents:
+### Phase 2: Delegate to Agents (USE TASK TOOL)

-**Always invoke** `resolution-verifier` if there are previous findings.
+**You MUST use the Task tool to invoke agents.** Simply saying "invoke resolution-verifier" does nothing - you must call the Task tool.

-**ALWAYS invoke** `finding-validator` for ALL unresolved findings from resolution-verifier.
-This is CRITICAL to prevent false positives from persisting.
+**If there are previous findings, invoke resolution-verifier FIRST:**

-**Invoke** `new-code-reviewer` if:
- Diff is substantial (>50 lines)
- Changes touch security-sensitive areas
- New files were added
- Complex logic was modified
+```
+Task(
+  subagent_type="resolution-verifier",
+  prompt="Verify resolution of these previous findings:\n\n[COPY THE PREVIOUS FINDINGS LIST HERE WITH IDs, FILES, LINES, AND DESCRIPTIONS]",
+  description="Verify previous findings resolved"
+)
+```

-**Invoke** `comment-analyzer` if:
- There are contributor comments since last review
- There are AI tool reviews to triage
- Questions remain unanswered
+**THEN invoke finding-validator for ALL unresolved findings:**
+
+```
+Task(
+  subagent_type="finding-validator",
+  prompt="Validate these unresolved findings:\n\n[COPY THE UNRESOLVED FINDINGS FROM RESOLUTION-VERIFIER]",
+  description="Validate unresolved findings"
+)
+```
+
+**Invoke new-code-reviewer if substantial changes:**
+
+```
+Task(
+  subagent_type="new-code-reviewer",
+  prompt="Review new code changes:\n\n[INCLUDE FILE LIST AND KEY CHANGES]",
+  description="Review new code"
+)
+```
+
+**Invoke comment-analyzer if there are comments:**
+
+```
+Task(
+  subagent_type="comment-analyzer",
+  prompt="Analyze these comments:\n\n[INCLUDE COMMENT LIST]",
+  description="Analyze comments"
+)
+```
+
+### Decision Matrix
+
+| Condition | Agent to Invoke |
+|-----------|-----------------|
+| Previous findings exist | `resolution-verifier` (ALWAYS) |
+| Unresolved findings exist | `finding-validator` (ALWAYS - MANDATORY) |
+| Diff > 50 lines | `new-code-reviewer` |
+| New comments exist | `comment-analyzer` |
+
+### Phase 3: Validate ALL Findings (MANDATORY)
+
+**⚠️ ABSOLUTE RULE: You MUST invoke finding-validator for EVERY finding, regardless of severity.**
+This includes unresolved findings from resolution-verifier AND any new findings from new-code-reviewer.
+- CRITICAL/HIGH/MEDIUM/LOW: ALL must be validated
+- There are NO exceptions — every finding the user sees must be independently verified
+
+After resolution-verifier and new-code-reviewer return their findings:
+1. **Batch findings for validation:**
+   - For ≤10 findings: Send all to finding-validator in one call
+   - For >10 findings: Group by file or category, invoke 2-4 validator calls in parallel
+   - This reduces overhead while maintaining thorough validation

-### Phase 3: Validate Unresolved Findings
-After resolution-verifier returns findings marked as unresolved:
-1. Pass ALL unresolved findings to finding-validator
 2. finding-validator will read the actual code at each location
 3. For each finding, it returns:
-   - `confirmed_valid`: Issue IS real → keep as unresolved
+   - `confirmed_valid`: Issue IS real → keep as finding
   - `dismissed_false_positive`: Original finding was WRONG → remove from findings
   - `needs_human_review`: Cannot determine → flag for human

+**Every finding in the final output MUST have:**
+- `validation_status`: One of "confirmed_valid" or "needs_human_review"
+- `validation_evidence`: The actual code snippet examined during validation
+- `validation_explanation`: Why the finding was confirmed or flagged
+
+**If any finding is missing validation_status in the final output, the review is INVALID.**
+
 ### Phase 4: Synthesize Results
 After all agents complete:
 1. Combine resolution verifications
@@ -177,9 +292,10 @@ After all agents complete:
 ## Cross-Validation

 When multiple agents report on the same area:
- **Agreement boosts confidence**: If resolution-verifier and new-code-reviewer both flag an issue, increase severity
+- **Agreement strengthens evidence**: If resolution-verifier and new-code-reviewer both flag an issue, this is strong signal
 - **Conflicts need resolution**: If agents disagree, investigate and document your reasoning
 - **Track consensus**: Note which findings have cross-agent validation
+- **Evidence-based, not confidence-based**: Multiple agents agreeing doesn't skip validation - all findings still verified

 ## Output Format

@@ -198,16 +314,14 @@ Provide your synthesis as a structured response matching the ParallelFollowupRes
      "validation_status": "confirmed_valid",
      "code_evidence": "const query = `SELECT * FROM users WHERE id = ${userId}`;",
      "line_range": [45, 45],
-      "explanation": "SQL injection is present - user input is concatenated...",
-      "confidence": 0.92
+      "explanation": "SQL injection is present - user input is concatenated directly into query"
    },
    {
      "finding_id": "QUAL-002",
      "validation_status": "dismissed_false_positive",
      "code_evidence": "const sanitized = DOMPurify.sanitize(data);",
      "line_range": [23, 26],
-      "explanation": "Original finding claimed XSS but code uses DOMPurify...",
-      "confidence": 0.88
+      "explanation": "Original finding claimed XSS but code uses DOMPurify for sanitization"
    }
  ],
  "new_findings": [...],
@@ -6,6 +6,83 @@ You are a focused logic and correctness review agent. You have been spawned by t

 Verify that the code logic is correct, handles all edge cases, and doesn't introduce subtle bugs. Focus ONLY on logic and correctness issues - not style, security, or general quality.

+## Phase 1: Understand the PR Intent (BEFORE Looking for Issues)
+
+**MANDATORY** - Before searching for issues, understand what this PR is trying to accomplish.
+
+1. **Read the provided context**
+   - PR description: What does the author say this does?
+   - Changed files: What areas of code are affected?
+   - Commits: How did the PR evolve?
+
+2. **Identify the change type**
+   - Bug fix: Correcting broken behavior
+   - New feature: Adding new capability
+   - Refactor: Restructuring without behavior change
+   - Performance: Optimizing existing code
+   - Cleanup: Removing dead code or improving organization
+
+3. **State your understanding** (include in your analysis)
+   ```
+   PR INTENT: This PR [verb] [what] by [how].
+   RISK AREAS: [what could go wrong specific to this change type]
+   ```
+
+**Only AFTER completing Phase 1, proceed to looking for issues.**
+
+Why this matters: Understanding intent prevents flagging intentional design decisions as bugs.
+
+## TRIGGER-DRIVEN EXPLORATION (CHECK YOUR DELEGATION PROMPT)
+
+**FIRST**: Check if your delegation prompt contains a `TRIGGER:` instruction.
+
+- **If TRIGGER is present** → Exploration is **MANDATORY**, even if the diff looks correct
+- **If no TRIGGER** → Use your judgment to explore or not
+
+### How to Explore (Bounded)
+
+1. **Read the trigger** - What pattern did the orchestrator identify?
+2. **Form the specific question** - "Do callers handle the new return type?" (not "what do callers do?")
+3. **Use Grep** to find call sites of the changed function/method
+4. **Use Read** to examine 3-5 callers
+5. **Answer the question** - Yes (report issue) or No (move on)
+6. **Stop** - Do not explore callers of callers (depth > 1)
+
+### Trigger-Specific Questions
+
+| Trigger | What to Check in Callers |
+|---------|-------------------------|
+| **Output contract changed** | Do callers assume the old return type/structure? |
+| **Input contract changed** | Do callers pass the old arguments/defaults? |
+| **Behavioral contract changed** | Does code after the call assume old ordering/timing? |
+| **Side effect removed** | Did callers depend on the removed effect? |
+| **Failure contract changed** | Can callers handle the new failure mode? |
+| **Null contract changed** | Do callers have explicit null checks or tri-state logic? |
+
+### Example Exploration
+
+```
+TRIGGER: Output contract changed (array → single object)
+QUESTION: Do callers use array methods?
+
+1. Grep for "getUserSettings(" → found 8 call sites
+2. Read dashboard.tsx:45 → uses .find() on result → ISSUE
+3. Read profile.tsx:23 → uses result.email directly → OK
+4. Read settings.tsx:67 → uses .map() on result → ISSUE
+5. STOP - Found 2 confirmed issues, pattern established
+
+FINDINGS:
+- dashboard.tsx:45 - uses .find() which doesn't exist on object
+- settings.tsx:67 - uses .map() which doesn't exist on object
+```
+
+### When NO Trigger is Given
+
+If the orchestrator doesn't specify a trigger, use your judgment:
+- Focus on the changed code first
+- Only explore callers if you suspect an issue from the diff
+- Don't explore "just to be thorough"
+
 ## CRITICAL: PR Scope and Context

 ### What IS in scope (report these issues):
@@ -140,6 +217,92 @@ Before reporting ANY finding, you MUST:

 **Your evidence must prove the issue exists - not just that you suspect it.**

+## Evidence Requirements (MANDATORY)
+
+Every finding you report MUST include a `verification` object with ALL of these fields:
+
+### Required Fields
+
+**code_examined** (string, min 1 character)
+The **exact code snippet** you examined. Copy-paste directly from the file:
+```
+CORRECT: "cursor.execute(f'SELECT * FROM users WHERE id={user_id}')"
+WRONG:   "SQL query that uses string interpolation"
+```
+
+**line_range_examined** (array of 2 integers)
+The exact line numbers [start, end] where the issue exists:
+```
+CORRECT: [45, 47]
+WRONG:   [1, 100]  // Too broad - you didn't examine all 100 lines
+```
+
+**verification_method** (one of these exact values)
+How you verified the issue:
+- `"direct_code_inspection"` - Found the issue directly in the code at the location
+- `"cross_file_trace"` - Traced through imports/calls to confirm the issue
+- `"test_verification"` - Verified through examination of test code
+- `"dependency_analysis"` - Verified through analyzing dependencies
+
+### Conditional Fields
+
+**is_impact_finding** (boolean, default false)
+Set to `true` ONLY if this finding is about impact on OTHER files (not the changed file):
+```
+TRUE:  "This change in utils.ts breaks the caller in auth.ts"
+FALSE: "This code in utils.ts has a bug" (issue is in the changed file)
+```
+
+**checked_for_handling_elsewhere** (boolean, default false)
+For ANY "missing X" claim (missing null check, missing bounds check, missing edge case handling):
+- Set `true` ONLY if you used Grep/Read tools to verify X is not handled elsewhere
+- Set `false` if you didn't search other files
+- **When true, include the search in your description:**
+  - "Searched `Grep('if.*null|!= null|\?\?', 'src/utils/')` - no null check found"
+  - "Checked callers via `Grep('processArray\(', '**/*.ts')` - none validate input"
+
+```
+TRUE:  "Searched for null checks in this file and callers - none found"
+FALSE: "This function should check for null" (didn't verify it's missing)
+```
+
+**If you cannot provide real evidence, you do not have a verified finding - do not report it.**
+
+**Search Before Claiming Absence:** Never claim a check is "missing" without searching for it first. Validation may exist in callers, guards, or type system constraints.
+
+## Valid Outputs
+
+Finding issues is NOT the goal. Accurate review is the goal.
+
+### Valid: No Significant Issues Found
+If the code is well-implemented, say so:
+```json
+{
+  "findings": [],
+  "summary": "Reviewed [files]. No logic issues found. The implementation correctly [positive observation about the code]."
+}
+```
+
+### Valid: Only Low-Severity Suggestions
+Minor improvements that don't block merge:
+```json
+{
+  "findings": [
+    {"severity": "low", "title": "Consider extracting magic number to constant", ...}
+  ],
+  "summary": "Code is sound. One minor suggestion for readability."
+}
+```
+
+### INVALID: Forced Issues
+Do NOT report issues just to have something to say:
+- Theoretical edge cases without evidence they're reachable
+- Style preferences not backed by project conventions
+- "Could be improved" without concrete problem
+- Pre-existing issues not introduced by this PR
+
+**Reporting nothing is better than reporting noise.** False positives erode trust faster than false negatives.
+
 ## Code Patterns to Flag

 ### Off-By-One Errors
@@ -217,6 +380,13 @@ Provide findings in JSON format:
    "description": "Loop uses `i < arr.length - 1` which skips the last element. For array [1, 2, 3], only processes [1, 2].",
    "category": "logic",
    "severity": "high",
+    "verification": {
+      "code_examined": "for (let i = 0; i < arr.length - 1; i++) { result.push(arr[i]); }",
+      "line_range_examined": [23, 25],
+      "verification_method": "direct_code_inspection"
+    },
+    "is_impact_finding": false,
+    "checked_for_handling_elsewhere": false,
    "example": {
      "input": "[1, 2, 3]",
      "actual_output": "Processes [1, 2]",
@@ -232,6 +402,13 @@ Provide findings in JSON format:
    "description": "Multiple async operations increment `count` without synchronization. With 10 concurrent increments, final count could be less than 10.",
    "category": "logic",
    "severity": "critical",
+    "verification": {
+      "code_examined": "await Promise.all(items.map(async () => { count++; }));",
+      "line_range_examined": [45, 47],
+      "verification_method": "direct_code_inspection"
+    },
+    "is_impact_finding": false,
+    "checked_for_handling_elsewhere": false,
    "example": {
      "input": "10 concurrent increments",
      "actual_output": "count might be 7, 8, or 9",
@@ -45,6 +45,7 @@ You have access to these specialized review agents via the Task tool:
 ### quality-reviewer
 **Description**: Code quality expert for complexity, duplication, error handling, maintainability, and pattern adherence.
 **When to use**: PRs with complex logic, large functions, new patterns, or significant business logic changes.
+**Special check**: If the PR adds similar logic in multiple files, flag it as a candidate for a shared utility.

 ### logic-reviewer
 **Description**: Logic and correctness specialist for algorithm verification, edge cases, state management, and race conditions.
@@ -62,9 +63,314 @@ You have access to these specialized review agents via the Task tool:
 **Description**: Finding validation specialist that re-investigates findings to confirm they are real issues, not false positives.
 **When to use**: After ALL specialist agents have reported their findings. Invoke for EVERY finding to validate it exists in the actual code.

+## CRITICAL: How to Invoke Specialist Agents
+
+**You MUST use the Task tool with the exact `subagent_type` names listed below.** Do NOT use `general-purpose` or any other built-in agent - always use our custom specialists.
+
+### Exact Agent Names (use these in subagent_type)
+
+| Agent | subagent_type value |
+|-------|---------------------|
+| Security reviewer | `security-reviewer` |
+| Quality reviewer | `quality-reviewer` |
+| Logic reviewer | `logic-reviewer` |
+| Codebase fit reviewer | `codebase-fit-reviewer` |
+| AI comment triage | `ai-triage-reviewer` |
+| Finding validator | `finding-validator` |
+
+### Task Tool Invocation Format
+
+When you invoke a specialist, use the Task tool like this:
+
+```
+Task(
+  subagent_type="security-reviewer",
+  prompt="This PR adds /api/login endpoint. Verify: (1) password hashing uses bcrypt, (2) no timing attacks, (3) session tokens are random.",
+  description="Security review of auth changes"
+)
+```
+
+### Example: Invoking Multiple Specialists in Parallel
+
+For a PR that adds authentication, invoke multiple agents in the SAME response:
+
+```
+Task(
+  subagent_type="security-reviewer",
+  prompt="This PR adds password auth to /api/login. Verify password hashing, timing attacks, token generation.",
+  description="Security review"
+)
+
+Task(
+  subagent_type="logic-reviewer",
+  prompt="This PR implements login with sessions. Check edge cases: empty password, wrong user, concurrent logins.",
+  description="Logic review"
+)
+
+Task(
+  subagent_type="quality-reviewer",
+  prompt="This PR adds auth code. Verify error messages don't leak info, no password logging.",
+  description="Quality review"
+)
+```
+
+### DO NOT USE
+
+- ❌ `general-purpose` - This is a generic built-in agent, NOT our specialist
+- ❌ `Explore` - This is for codebase exploration, NOT for PR review
+- ❌ `Plan` - This is for planning, NOT for PR review
+
+**Always use our specialist agents** (`security-reviewer`, `logic-reviewer`, `quality-reviewer`, `codebase-fit-reviewer`, `ai-triage-reviewer`, `finding-validator`) for PR review tasks.
+
 ## Your Workflow

-### Phase 1: Analysis
+### Phase 0: Understand the PR Holistically (BEFORE Delegation)
+
+**MANDATORY** - Before invoking ANY specialist agent, you MUST understand what this PR is trying to accomplish.
+
+1. **Check for Merge Conflicts FIRST** - If `has_merge_conflicts` is `true` in the PR context:
+   - Add a CRITICAL finding immediately
+   - Include in your PR UNDERSTANDING output: "⚠️ MERGE CONFLICTS: PR cannot be merged until resolved"
+   - Still proceed with review (conflicts don't skip the review)
+
+2. **Read the PR Description** - What is the stated goal?
+3. **Review the Commit Timeline** - How did the PR evolve? Were issues fixed in later commits?
+4. **Examine Related Files** - What tests, imports, and dependents are affected?
+5. **Identify the PR Intent** - Bug fix? Feature? Refactor? Breaking change?
+
+**Create a mental model:**
+- "This PR [adds/fixes/refactors] X by [changing] Y, which is [used by/depends on] Z"
+- Identify what COULD go wrong based on the change type
+
+**Output your synthesis before delegating:**
+```
+PR UNDERSTANDING:
+- Intent: [one sentence describing what this PR does]
+- Critical changes: [2-3 most important files and what changed]
+- Risk areas: [security, logic, breaking changes, etc.]
+- Files to verify: [related files that might be impacted]
+```
+
+**Only AFTER completing Phase 0, proceed to Phase 1 (Trigger Detection).**
+
+## What the Diff Is For
+
+**The diff is the question, not the answer.**
+
+The code changes show what the author is asking you to review. Before delegating to specialists:
+
+### Answer These Questions
+1. **What is this diff trying to accomplish?**
+   - Read the PR description
+   - Look at the file names and change patterns
+   - Understand the author's intent
+
+2. **What could go wrong with this approach?**
+   - Security: Does it handle user input? Auth? Secrets?
+   - Logic: Are there edge cases? State changes? Async issues?
+   - Quality: Is it maintainable? Does it follow patterns?
+   - Fit: Does it reinvent existing utilities?
+
+3. **What should specialists verify?**
+   - Specific concerns, not generic "check for bugs"
+   - Files to examine beyond the changed files
+   - Questions the diff raises but doesn't answer
+
+### Delegate with Context
+
+When invoking specialists, include:
+- Your synthesis of what the PR does
+- Specific concerns to investigate
+- Related files they should examine
+
+**Never delegate blind.** "Review this code" without context leads to noise. "This PR adds user auth - verify password hashing and session management" leads to signal.
+
+## MANDATORY EXPLORATION TRIGGERS (Language-Agnostic)
+
+**CRITICAL**: Certain change patterns ALWAYS require checking callers/dependents, even if the diff looks correct. The issue may only be visible in how OTHER code uses the changed code.
+
+When you identify these patterns in the diff, instruct specialists to explore direct callers:
+
+### 1. OUTPUT CONTRACT CHANGED
+**Detect:** Function/method returns different value, type, or structure than before
+- Return type changed (array → single item, nullable → non-null, wrapped → unwrapped)
+- Return value semantics changed (empty array vs null, false vs undefined)
+- Structure changed (object shape different, fields added/removed)
+
+**Instruct specialists:** "Check how callers USE the return value. Look for operations that assume the old structure."
+
+**Stop when:** Checked 3-5 direct callers OR found a confirmed issue
+
+### 2. INPUT CONTRACT CHANGED
+**Detect:** Parameters added, removed, reordered, or defaults changed
+- New required parameters
+- Default parameter values changed
+- Parameter types changed
+
+**Instruct specialists:** "Find callers that don't pass [parameter] - they rely on the old default. Check callers passing arguments in the old order."
+
+**Stop when:** Identified implicit callers (those not passing the changed parameter)
+
+### 3. BEHAVIORAL CONTRACT CHANGED
+**Detect:** Same inputs/outputs but different internal behavior
+- Operations reordered (sequential → parallel, different order)
+- Timing changed (sync → async, immediate → deferred)
+- Performance characteristics changed (O(1) → O(n), single query → N+1)
+
+**Instruct specialists:** "Check if code AFTER the call assumes the old behavior (ordering, timing, completion)."
+
+**Stop when:** Verified 3-5 call sites for ordering dependencies
+
+### 4. SIDE EFFECT CONTRACT CHANGED
+**Detect:** Observable effects added or removed
+- No longer writes to cache/database/file
+- No longer emits events/notifications
+- No longer cleans up related resources (sessions, connections)
+
+**Instruct specialists:** "Check if callers depended on the removed effect. Verify replacement mechanism actually exists."
+
+**Stop when:** Confirmed callers don't depend on removed effect OR found dependency
+
+### 5. FAILURE CONTRACT CHANGED
+**Detect:** How the function handles errors changed
+- Now throws/returns error where it didn't before (permissive → strict)
+- Now succeeds silently where it used to fail (strict → permissive)
+- Different error type/code returned
+- Return value changes on failure (e.g., `return true` → `return false`, `return null` → `throw Error`)
+
+**Examples:**
+- `validateEmail()` used to return `true` on service error (permissive), now returns `false` (strict)
+- `processPayment()` used to throw on failure, now returns `{success: false, error: ...}` (different failure mode)
+- `fetchUser()` used to return `null` for not-found, now throws `NotFoundError` (exception vs return value)
+
+**Instruct specialists:** "Check if callers can handle the new failure mode. Look for missing error handling in critical paths. Verify callers don't assume the old success/failure behavior."
+
+**Stop when:** Verified caller resilience OR found unhandled failure case
+
+### 6. NULL/UNDEFINED CONTRACT CHANGED
+**Detect:** Null handling changed
+- Now returns null where it returned a value before
+- Now returns a value where it returned null before
+- Null checks added or removed
+
+**Instruct specialists:** "Find callers with explicit null checks (`=== null`, `!= null`). Check for tri-state logic (true/false/null as different states)."
+
+**Stop when:** Checked callers for null-dependent logic
+
+### Phase 1: Detect Semantic Change Patterns (MANDATORY)
+
+**MANDATORY** - After understanding the PR, you MUST analyze the diff for semantic contract changes before delegating to ANY specialist.
+
+**For EACH changed function, method, or component in the diff, check:**
+
+1. Does it return something different? → **OUTPUT CONTRACT CHANGED**
+2. Do its parameters/defaults change? → **INPUT CONTRACT CHANGED**
+3. Does it behave differently internally? → **BEHAVIORAL CONTRACT CHANGED**
+4. Were side effects added or removed? → **SIDE EFFECT CONTRACT CHANGED**
+5. Does it handle errors differently? → **FAILURE CONTRACT CHANGED**
+6. Did null/undefined handling change? → **NULL CONTRACT CHANGED**
+
+**Output your analysis explicitly:**
+```
+TRIGGER DETECTION:
+- getUserSettings(): OUTPUT CONTRACT CHANGED (returns object instead of array)
+- processOrder(): BEHAVIORAL CONTRACT CHANGED (sequential → parallel execution)
+- validateInput(): NO TRIGGERS (internal logic change only, same contract)
+```
+
+**If NO triggers apply:**
+```
+TRIGGER DETECTION: No semantic contract changes detected.
+Changes are internal-only (logic, style, CSS, refactor without API changes).
+```
+
+**This phase is MANDATORY. Do not skip it even for "simple" PRs.**
+
+## ENFORCEMENT: Required Output Before Delegation
+
+**You CANNOT invoke the Task tool until you have output BOTH Phase 0 and Phase 1.**
+
+Your response MUST include these sections BEFORE any Task tool invocation:
+
+```
+PR UNDERSTANDING:
+- Intent: [one sentence describing what this PR does]
+- Critical changes: [2-3 most important files and what changed]
+- Risk areas: [security, logic, breaking changes, etc.]
+- Files to verify: [related files that might be impacted]
+
+TRIGGER DETECTION:
+- [function1](): [TRIGGER_TYPE] (description) OR NO TRIGGERS
+- [function2](): [TRIGGER_TYPE] (description) OR NO TRIGGERS
+...
+```
+
+**Why this is enforced:** Without understanding intent, specialists receive context-free code and produce false positives. Without trigger detection, contract-breaking changes slip through because "the diff looks fine."
+
+**Only AFTER outputting both sections, proceed to Phase 2 (Analysis).**
+
+### Trigger Detection Examples
+
+**Function signature change:**
+```
+TRIGGER DETECTION:
+- getUser(id): INPUT CONTRACT CHANGED (added optional `options` param with default)
+- getUser(id): OUTPUT CONTRACT CHANGED (returns User instead of User[])
+```
+
+**Error handling change:**
+```
+TRIGGER DETECTION:
+- validateEmail(): FAILURE CONTRACT CHANGED (now returns false on service error instead of true)
+```
+
+**Refactor with no contract change:**
+```
+TRIGGER DETECTION: No semantic contract changes detected.
+extractHelper() is a new internal function, no existing callers.
+processData() internal logic changed but input/output contract is identical.
+```
+
+### How Triggers Flow to Specialists (MANDATORY)
+
+**CRITICAL: When triggers ARE detected, you MUST include them in delegation prompts.**
+
+This is NOT optional. Every Task invocation MUST follow this checklist:
+
+**Pre-Delegation Checklist (verify before EACH Task call):**
+```
+□ Does the prompt include PR intent summary?
+□ Does the prompt include specific concerns to verify?
+□ If triggers were detected → Does the prompt include "TRIGGER: [TYPE] - [description]"?
+□ If triggers were detected → Does the prompt include "Stop when: [condition]"?
+□ Are known callers/dependents included (if available in PR context)?
+```
+
+**Required Format When Triggers Exist:**
+```
+Task(
+  subagent_type="logic-reviewer",
+  prompt="This PR changes getUserSettings() to return a single object instead of an array.
+
+          TRIGGER: OUTPUT CONTRACT CHANGED - returns object instead of array
+          EXPLORATION REQUIRED: Check 3-5 direct callers for array method usage (.map, .filter, .find, .forEach).
+          Stop when: Found callers using array methods OR verified 5 callers handle it correctly.
+
+          Known callers: [list from PR context if available]",
+  description="Logic review - output contract change"
+)
+```
+
+**If you detect triggers in Phase 1 but don't pass them to specialists, the review is INCOMPLETE.**
+
+### Exploration Boundaries
+
+❌ Explore because "I want to be thorough"
+❌ Check callers of callers (depth > 1) unless a confirmed issue needs tracing
+❌ Keep exploring after the trigger-specific question is answered
+❌ Skip exploration because "the diff looks fine" - triggers override this
+
+### Phase 2: Analysis

 Analyze the PR thoroughly:

@@ -73,7 +379,7 @@ Analyze the PR thoroughly:
 3. **Identify Risk Areas**: Security-sensitive? Complex logic? New patterns?
 4. **Check for AI Comments**: Are there existing AI reviewer comments to triage?

-### Phase 2: Delegation
+### Phase 3: Delegation

 Based on your analysis, invoke the appropriate specialist agents. You can invoke multiple agents in parallel by calling the Task tool multiple times in the same response.

@@ -87,36 +393,124 @@ Based on your analysis, invoke the appropriate specialist agents. You can invoke
 - **New patterns/large additions**: Always invoke codebase-fit-reviewer.
 - **Existing AI comments**: Always invoke ai-triage-reviewer.

-**Example delegation**:
+**Context-Rich Delegation (CRITICAL):**
+
+When you invoke a specialist, your prompt to them MUST include:
+
+1. **PR Intent Summary** - One sentence from your Phase 0 synthesis
+   - Example: "This PR adds JWT authentication to the API endpoints"
+
+2. **Specific Concerns** - What you want them to verify
+   - Security: "Verify token validation, check for secret exposure"
+   - Logic: "Check for race conditions in token refresh"
+   - Quality: "Verify error handling in auth middleware"
+   - Fit: "Check if existing auth helpers were considered"
+
+3. **Files of Interest** - Beyond just the changed files
+   - "Also examine tests/auth.test.ts for coverage gaps"
+   - "Check if utils/crypto.ts has relevant helpers"
+
+4. **Trigger Instructions** (from Phase 1) - **MANDATORY if triggers were detected:**
+   - "TRIGGER: [TYPE] - [description of what changed]"
+   - "EXPLORATION REQUIRED: [what to check in callers]"
+   - "Stop when: [condition to stop exploring]"
+   - **You MUST include ALL THREE lines for each trigger**
+   - If no triggers were detected in Phase 1, you may omit this section.
+
+5. **Known Callers/Dependents** (from PR context) - If the PR context includes related files:
+   - Include any known callers of the changed functions
+   - Include files that import/depend on the changed files
+   - Example: "Known callers: dashboard.tsx:45, settings.tsx:67, api/users.ts:23"
+   - This gives specialists starting points for exploration instead of searching blind
+
+**Anti-pattern:** "Review src/auth/login.ts for security issues"
+**Good pattern:** "This PR adds password-based login. Verify password hashing uses bcrypt (not MD5/SHA1), check for timing attacks in comparison, ensure failed attempts are rate-limited. Also check if existing RateLimiter in utils/ was considered."
+
+**Example delegation with triggers and known callers:**
+
 ```
-For a PR adding a new authentication endpoint:
- Invoke security-reviewer for auth logic
- Invoke quality-reviewer for code structure
- Invoke logic-reviewer for edge cases in auth flow
+Task(
+  subagent_type="logic-reviewer",
+  prompt="This PR changes getUserSettings() to return a single object instead of an array.
+          TRIGGER: Output contract changed.
+          Check 3-5 direct callers for array method usage (.map, .filter, .find, .forEach).
+          Stop when: Found callers using array methods OR verified 5 callers handle it correctly.
+          Known callers from PR context: dashboard.tsx:45, settings.tsx:67, components/UserPanel.tsx:89
+          Also verify edge cases in the new implementation.",
+  description="Logic review - output contract change"
+)
 ```

-### Phase 3: Synthesis
+**Example delegation without triggers:**
+
+```
+Task(
+  subagent_type="security-reviewer",
+  prompt="This PR adds /api/login endpoint with password auth. Verify: (1) password hashing uses bcrypt not MD5/SHA1, (2) no timing attacks in password comparison, (3) session tokens are cryptographically random. Also check utils/crypto.ts for existing helpers.",
+  description="Security review of auth endpoint"
+)
+
+Task(
+  subagent_type="quality-reviewer",
+  prompt="This PR adds auth code. Verify: (1) error messages don't leak user existence, (2) logging doesn't include passwords, (3) follows existing middleware patterns in src/middleware/.",
+  description="Quality review of auth code"
+)
+```
+
+### Phase 4: Synthesis

 After receiving agent results, synthesize findings:

-1. **Aggregate**: Collect all findings from all agents
+1. **Aggregate**: Collect ALL findings from all agents (no filtering at this stage!)
 2. **Cross-validate** (see "Multi-Agent Agreement" section):
   - Group findings by (file, line, category)
-   - If 2+ agents report same issue → merge into one, boost confidence by +0.15
+   - If 2+ agents report same issue → merge into one finding
   - Set `cross_validated: true` and populate `source_agents` list
   - Track agreed finding IDs in `agent_agreement.agreed_findings`
 3. **Deduplicate**: Remove overlapping findings (same file + line + issue type)
-4. **Route by Confidence** (see "Confidence Tiers" section):
-   - HIGH (>=0.8): Include as-is
-   - MEDIUM (0.5-0.8): Include with "[Potential]" prefix
-   - LOW (<0.5): Log and exclude
-5. **Generate Verdict**: Based on severity of remaining findings
+4. **Send ALL to Validator**: Every finding goes to finding-validator (see Phase 4.5)
+   - Do NOT filter by confidence before validation
+   - Do NOT drop "low confidence" findings
+   - The validator determines what's real, not the orchestrator
+5. **Generate Verdict**: Based on VALIDATED findings only

-### Phase 3.5: Finding Validation (CRITICAL - Prevent False Positives)
+### Phase 4.5: Finding Validation (CRITICAL - Prevent False Positives)

-**MANDATORY STEP** - After synthesis, validate ALL findings before generating verdict:
+**MANDATORY STEP** - After synthesis, validate ALL findings before generating verdict.
+
+**⚠️ ABSOLUTE RULE: You MUST invoke finding-validator for EVERY finding, regardless of severity.**
+- CRITICAL findings: MUST validate
+- HIGH findings: MUST validate
+- MEDIUM findings: MUST validate
+- LOW findings: MUST validate
+- Style suggestions: MUST validate
+
+There are NO exceptions. A LOW-severity finding that is a false positive is still noise for the developer. Every finding the user sees must have been independently verified against the actual code. Do NOT skip validation for any finding — not for "obvious" ones, not for "style" ones, not for "low-risk" ones. If it appears in the findings array, it must have a `validation_status`.
+
+1. **Invoke finding-validator** for findings from specialist agents:
+
+   **For small PRs (≤10 findings):** Invoke validator once with ALL findings in a single prompt.
+
+   **For large PRs (>10 findings):** Batch findings by file or category:
+   - Group findings in the same file together (validator can read file once)
+   - Group findings of the same category together (security, quality, logic)
+   - Invoke 2-4 validator calls in parallel, each handling a batch
+
+   **Example batch invocation:**
+   ```
+   Task(
+     subagent_type="finding-validator",
+     prompt="Validate these 5 findings in src/auth/:\n
+             1. SEC-001: SQL injection at login.ts:45\n
+             2. SEC-002: Hardcoded secret at config.ts:12\n
+             3. QUAL-001: Missing error handling at login.ts:78\n
+             4. QUAL-002: Code duplication at auth.ts:90\n
+             5. LOGIC-001: Off-by-one at validate.ts:23\n
+             Read the actual code and validate each. Return a validation result for EACH finding.",
+     description="Validate auth-related findings batch"
+   )
+   ```

-1. **Invoke finding-validator** for EACH finding from specialist agents
 2. For each finding, the validator returns one of:
   - `confirmed_valid` - Issue IS real, keep in findings list
   - `dismissed_false_positive` - Original finding was WRONG, remove from findings
@@ -131,65 +525,92 @@ After receiving agent results, synthesize findings:
   - A finding dismissed as false positive does NOT count toward verdict
   - Only confirmed issues determine severity

-**Why this matters:** Specialist agents sometimes flag issues that don't exist in the actual code. The validator reads the code with fresh eyes to catch these false positives before they're reported.
+5. **Every finding in the final output MUST have:**
+   - `validation_status`: One of "confirmed_valid" or "needs_human_review"
+   - `validation_evidence`: The actual code snippet examined during validation
+   - `validation_explanation`: Why the finding was confirmed or flagged
+
+**If any finding is missing validation_status in the final output, the review is INVALID.**
+
+**Why this matters:** Specialist agents sometimes flag issues that don't exist in the actual code. The validator reads the code with fresh eyes to catch these false positives before they're reported. This applies to ALL severity levels — a LOW false positive wastes developer time just like a HIGH one.

 **Example workflow:**
 ```
-Specialist finds 3 issues → finding-validator validates each →
-Result: 2 confirmed, 1 dismissed → Verdict based on 2 issues
+Specialist finds 3 issues (1 MEDIUM, 2 LOW) → finding-validator validates ALL 3 →
+Result: 2 confirmed, 1 dismissed → Verdict based on 2 validated issues
 ```

-## Confidence Tiers
+**Example validation invocation:**
+```
+Task(
+  subagent_type="finding-validator",
+  prompt="Validate this finding: 'SQL injection in user lookup at src/auth/login.ts:45'. Read the actual code at that location and determine if the issue exists. Return confirmed_valid, dismissed_false_positive, or needs_human_review.",
+  description="Validate SQL injection finding"
+)
+```

-After validation, findings are routed based on confidence scores:
+## Evidence-Based Validation (NOT Confidence-Based)

-| Tier | Score Range | Treatment |
-|------|-------------|-----------|
-| HIGH | >= 0.8 | Included as reported, affects verdict |
-| MEDIUM | 0.5 - 0.8 | Included with "[Potential]" prefix, affects verdict |
-| LOW | < 0.5 | Logged for monitoring, excluded from output |
+**CRITICAL: This system does NOT use confidence scores to filter findings.**

-**Guidelines for assigning confidence:**
- 0.9+ : Direct evidence in code, multiple indicators, clear violation
- 0.8-0.9 : Strong evidence, clear pattern, high certainty
- 0.6-0.8 : Likely issue but some uncertainty, may need context
- 0.4-0.6 : Possible issue, limited evidence, context-dependent
- < 0.4 : Speculation, no direct evidence, likely false positive
+All findings are validated against actual code. The validator determines what's real:
+
+| Validation Status | Meaning | Treatment |
+|-------------------|---------|-----------|
+| `confirmed_valid` | Evidence proves issue EXISTS | Include in findings |
+| `dismissed_false_positive` | Evidence proves issue does NOT exist | Move to `dismissed_findings` |
+| `needs_human_review` | Evidence is ambiguous | Include with flag for human |
+
+**Why evidence-based, not confidence-based:**
+- A "90% confidence" finding can be WRONG (false positive)
+- A "70% confidence" finding can be RIGHT (real issue)
+- Only actual code examination determines validity
+- Confidence scores are subjective; evidence is objective
+
+**What the validator checks:**
+1. Does the problematic code actually exist at the stated location?
+2. Is there mitigation elsewhere that the specialist missed?
+3. Does the finding accurately describe what the code does?
+4. Is this a real issue or a misunderstanding of intent?

 **Example:**
- SQL injection with `userId` in query string: 0.95 (direct evidence)
- Missing null check where input could be null: 0.75 (likely but depends on callers)
- "This might cause issues" without specifics: 0.3 (speculation, will be dropped)
+```
+Specialist claims: "SQL injection at line 45"
+Validator reads line 45, finds: parameterized query with $1 placeholder
+Result: dismissed_false_positive - "Code uses parameterized queries, not string concat"
+```

 ## Multi-Agent Agreement

 When multiple specialist agents flag the same issue (same file + line + category), this is strong signal:

-### Confidence Boost
- If 2+ agents agree: confidence boosted by +0.15 (max 0.95)
- cross_validated field set to true
- source_agents lists all agents that flagged the issue
+### Cross-Validation Signal
+- If 2+ agents independently find the same issue → stronger evidence
+- Set `cross_validated: true` on the merged finding
+- Populate `source_agents` with all agents that flagged it
+- This doesn't skip validation - validator still checks the code

 ### Why This Matters
- Independent verification increases certainty
+- Independent verification from different perspectives
 - False positives rarely get flagged by multiple specialized agents
- Multi-agent agreement often indicates real issues
+- Helps prioritize which findings to fix first

 ### Example
 ```
-security-reviewer finds: XSS vulnerability at line 45 (confidence: 0.75)
-quality-reviewer finds: Unsafe string interpolation at line 45 (confidence: 0.70)
+security-reviewer finds: XSS vulnerability at line 45
+quality-reviewer finds: Unsafe string interpolation at line 45

-Result: Single finding with confidence 0.90 (0.75 + 0.15 boost)
+Result: Single finding merged
        source_agents: ["security-reviewer", "quality-reviewer"]
        cross_validated: true
+        → Still sent to validator for evidence-based confirmation
 ```

 ### Agent Agreement Tracking
 The `agent_agreement` field in structured output tracks:
- `agreed_findings`: Finding IDs where 2+ agents agreed
- `conflicting_findings`: Finding IDs where agents disagreed (reserved for future)
- `resolution_notes`: How conflicts were resolved (reserved for future)
+- `agreed_findings`: Finding IDs where 2+ agents agreed (stronger evidence)
+- `conflicting_findings`: Finding IDs where agents disagreed
+- `resolution_notes`: How conflicts were resolved

 **Note:** Agent agreement data is logged for monitoring. The cross-validation results
 are reflected in each finding's source_agents, cross_validated, and confidence fields.
@@ -203,7 +624,7 @@ After synthesis and validation, output your final review in this JSON format:
  "analysis_summary": "Brief description of what you analyzed and why you chose those agents",
  "agents_invoked": ["security-reviewer", "quality-reviewer", "finding-validator"],
  "validation_summary": {
-    "total_findings": 5,
+    "total_findings_from_specialists": 5,
    "confirmed_valid": 3,
    "dismissed_false_positive": 2,
    "needs_human_review": 0
@@ -218,7 +639,6 @@ After synthesis and validation, output your final review in this JSON format:
      "description": "User input directly interpolated into SQL query",
      "category": "security",
      "severity": "critical",
-      "confidence": 0.95,
      "suggested_fix": "Use parameterized queries",
      "fixable": true,
      "source_agents": ["security-reviewer"],
@@ -227,6 +647,17 @@ After synthesis and validation, output your final review in this JSON format:
      "validation_evidence": "Actual code: `const query = 'SELECT * FROM users WHERE id = ' + userId`"
    }
  ],
+  "dismissed_findings": [
+    {
+      "id": "finding-2",
+      "original_title": "Timing attack in token comparison",
+      "original_severity": "low",
+      "original_file": "src/auth/token.ts",
+      "original_line": 120,
+      "dismissal_reason": "Validator found this is a cache check, not authentication decision",
+      "validation_evidence": "Code at line 120: `if (cachedToken === newToken) return cached;` - Only affects caching, not auth"
+    }
+  ],
  "agent_agreement": {
    "agreed_findings": ["finding-1", "finding-3"],
    "conflicting_findings": [],
@@ -237,12 +668,18 @@ After synthesis and validation, output your final review in this JSON format:
 }
 ```

-**Note on validation fields:**
- `validation_summary` at top level tracks validation statistics
- Each finding includes `validation_status` ("confirmed_valid", "dismissed_false_positive", or "needs_human_review")
- Each finding includes `validation_evidence` with actual code snippet from validation
- Only include findings with `validation_status: "confirmed_valid"` or `"needs_human_review"` in the final output
- Dismissed findings should be removed from the findings array entirely
+**CRITICAL: Transparency Requirements**
+- `findings` array: Contains ONLY `confirmed_valid` and `needs_human_review` findings
+- `dismissed_findings` array: Contains ALL findings that were validated and dismissed as false positives
+  - Users can see what was investigated and why it was dismissed
+  - This prevents hidden filtering and builds trust
+- `validation_summary`: Counts must match: `total = confirmed + dismissed + needs_human_review`
+
+**Evidence-Based Validation:**
+- Every finding in `findings` MUST have `validation_status` and `validation_evidence`
+- Every entry in `dismissed_findings` MUST have `dismissal_reason` and `validation_evidence`
+- If a specialist reported something, it MUST appear in either `findings` OR `dismissed_findings`
+- Nothing should silently disappear

 ## Verdict Types (Strict Quality Gates)

@@ -261,13 +698,15 @@ We use strict quality gates because AI can fix issues quickly. Only LOW severity

 ## Key Principles

-1. **YOU Decide**: No hardcoded rules - you analyze and choose agents based on content
-2. **Parallel Execution**: Invoke multiple agents in the same turn for speed
-3. **Thoroughness**: Every PR deserves analysis - never skip because it "looks simple"
-4. **Cross-Validation**: Multiple agents agreeing increases confidence
-5. **High Confidence**: Only report findings with ≥80% confidence
-6. **Actionable**: Every finding must have a specific, actionable fix
-7. **Project Agnostic**: Works for any project type - backend, frontend, fullstack, any language
+1. **Understand First**: Never delegate until you understand PR intent - findings without context lead to false positives
+2. **YOU Decide**: No hardcoded rules - you analyze and choose agents based on content
+3. **Parallel Execution**: Invoke multiple agents in the same turn for speed
+4. **Thoroughness**: Every PR deserves analysis - never skip because it "looks simple"
+5. **Cross-Validation**: Multiple agents agreeing strengthens evidence
+6. **Evidence-Based**: Every finding must be validated against actual code - no filtering by "confidence"
+7. **Transparent**: Include dismissed findings in output so users see complete picture
+8. **Actionable**: Every finding must have a specific, actionable fix
+9. **Project Agnostic**: Works for any project type - backend, frontend, fullstack, any language

 ## Remember

@@ -6,6 +6,82 @@ You are a focused code quality review agent. You have been spawned by the orches

 Perform a thorough code quality review of the provided code changes. Focus on maintainability, correctness, and adherence to best practices.

+## Phase 1: Understand the PR Intent (BEFORE Looking for Issues)
+
+**MANDATORY** - Before searching for issues, understand what this PR is trying to accomplish.
+
+1. **Read the provided context**
+   - PR description: What does the author say this does?
+   - Changed files: What areas of code are affected?
+   - Commits: How did the PR evolve?
+
+2. **Identify the change type**
+   - Bug fix: Correcting broken behavior
+   - New feature: Adding new capability
+   - Refactor: Restructuring without behavior change
+   - Performance: Optimizing existing code
+   - Cleanup: Removing dead code or improving organization
+
+3. **State your understanding** (include in your analysis)
+   ```
+   PR INTENT: This PR [verb] [what] by [how].
+   RISK AREAS: [what could go wrong specific to this change type]
+   ```
+
+**Only AFTER completing Phase 1, proceed to looking for issues.**
+
+Why this matters: Understanding intent prevents flagging intentional design decisions as bugs.
+
+## TRIGGER-DRIVEN EXPLORATION (CHECK YOUR DELEGATION PROMPT)
+
+**FIRST**: Check if your delegation prompt contains a `TRIGGER:` instruction.
+
+- **If TRIGGER is present** → Exploration is **MANDATORY**, even if the diff looks correct
+- **If no TRIGGER** → Use your judgment to explore or not
+
+### How to Explore (Bounded)
+
+1. **Read the trigger** - What pattern did the orchestrator identify?
+2. **Form the specific question** - "Do callers handle error cases from this function?" (not "what do callers do?")
+3. **Use Grep** to find call sites of the changed function/method
+4. **Use Read** to examine 3-5 callers
+5. **Answer the question** - Yes (report issue) or No (move on)
+6. **Stop** - Do not explore callers of callers (depth > 1)
+
+### Quality-Specific Trigger Questions
+
+| Trigger | Quality Question to Answer |
+|---------|---------------------------|
+| **Output contract changed** | Do callers have proper type handling for the new return type? |
+| **Behavioral contract changed** | Does the timing change cause callers to have race conditions or stale data? |
+| **Side effect removed** | Do callers now need to handle what the function used to do automatically? |
+| **Failure contract changed** | Do callers have proper error handling for the new failure mode? |
+| **Performance changed** | Do callers operate at scale where the performance change compounds? |
+
+### Example Exploration
+
+```
+TRIGGER: Behavioral contract changed (sequential → parallel operations)
+QUESTION: Do callers depend on the old sequential ordering?
+
+1. Grep for "processOrder(" → found 6 call sites
+2. Read checkout.ts:89 → reads database immediately after call → ISSUE (race condition)
+3. Read batch-job.ts:34 → awaits and then processes result → OK
+4. Read api/orders.ts:56 → sends confirmation after call → ISSUE (email before DB write)
+5. STOP - Found 2 quality issues
+
+FINDINGS:
+- checkout.ts:89 - Race condition: reads from DB before parallel write completes
+- api/orders.ts:56 - Email sent before order is persisted (ordering dependency broken)
+```
+
+### When NO Trigger is Given
+
+If the orchestrator doesn't specify a trigger, use your judgment:
+- Focus on quality issues in the changed code first
+- Only explore callers if you suspect an issue from the diff
+- Don't explore "just to be thorough"
+
 ## CRITICAL: PR Scope and Context

 ### What IS in scope (report these issues):
@@ -44,6 +120,7 @@ Perform a thorough code quality review of the provided code changes. Focus on ma
 - **Copy-Paste Code**: Similar functions with minor differences
 - **Redundant Implementations**: Re-implementing existing functionality
 - **Should Use Library**: Reinventing standard functionality
+- **PR-Internal Duplication**: Same new logic added to multiple files in this PR (should be a shared utility)

 ### 4. Maintainability
 - **Magic Numbers**: Hardcoded numbers without explanation
@@ -141,6 +218,92 @@ Before reporting ANY finding, you MUST:

 **Your evidence must prove the issue exists - not just that you suspect it.**

+## Evidence Requirements (MANDATORY)
+
+Every finding you report MUST include a `verification` object with ALL of these fields:
+
+### Required Fields
+
+**code_examined** (string, min 1 character)
+The **exact code snippet** you examined. Copy-paste directly from the file:
+```
+CORRECT: "cursor.execute(f'SELECT * FROM users WHERE id={user_id}')"
+WRONG:   "SQL query that uses string interpolation"
+```
+
+**line_range_examined** (array of 2 integers)
+The exact line numbers [start, end] where the issue exists:
+```
+CORRECT: [45, 47]
+WRONG:   [1, 100]  // Too broad - you didn't examine all 100 lines
+```
+
+**verification_method** (one of these exact values)
+How you verified the issue:
+- `"direct_code_inspection"` - Found the issue directly in the code at the location
+- `"cross_file_trace"` - Traced through imports/calls to confirm the issue
+- `"test_verification"` - Verified through examination of test code
+- `"dependency_analysis"` - Verified through analyzing dependencies
+
+### Conditional Fields
+
+**is_impact_finding** (boolean, default false)
+Set to `true` ONLY if this finding is about impact on OTHER files (not the changed file):
+```
+TRUE:  "This change in utils.ts breaks the caller in auth.ts"
+FALSE: "This code in utils.ts has a bug" (issue is in the changed file)
+```
+
+**checked_for_handling_elsewhere** (boolean, default false)
+For ANY "missing X" claim (missing error handling, missing validation, missing null check):
+- Set `true` ONLY if you used Grep/Read tools to verify X is not handled elsewhere
+- Set `false` if you didn't search other files
+- **When true, include the search in your description:**
+  - "Searched `Grep('try.*catch|\.catch\(', 'src/auth/')` - no error handling found"
+  - "Checked callers via `Grep('processPayment\(', '**/*.ts')` - none handle errors"
+
+```
+TRUE:  "Searched for try/catch patterns in this file and callers - none found"
+FALSE: "This function should have error handling" (didn't verify it's missing)
+```
+
+**If you cannot provide real evidence, you do not have a verified finding - do not report it.**
+
+**Search Before Claiming Absence:** Never claim something is "missing" without searching for it first. If you claim there's no error handling, show the search that confirmed its absence.
+
+## Valid Outputs
+
+Finding issues is NOT the goal. Accurate review is the goal.
+
+### Valid: No Significant Issues Found
+If the code is well-implemented, say so:
+```json
+{
+  "findings": [],
+  "summary": "Reviewed [files]. No quality issues found. The implementation correctly [positive observation about the code]."
+}
+```
+
+### Valid: Only Low-Severity Suggestions
+Minor improvements that don't block merge:
+```json
+{
+  "findings": [
+    {"severity": "low", "title": "Consider extracting magic number to constant", ...}
+  ],
+  "summary": "Code is sound. One minor suggestion for readability."
+}
+```
+
+### INVALID: Forced Issues
+Do NOT report issues just to have something to say:
+- Theoretical edge cases without evidence they're reachable
+- Style preferences not backed by project conventions
+- "Could be improved" without concrete problem
+- Pre-existing issues not introduced by this PR
+
+**Reporting nothing is better than reporting noise.** False positives erode trust faster than false negatives.
+
 ## Code Patterns to Flag

 ### JavaScript/TypeScript
@@ -238,6 +401,13 @@ Provide findings in JSON format:
    "description": "The paymentGateway.charge() call is async but has no error handling. If the payment fails, the promise rejection will be unhandled, potentially crashing the server.",
    "category": "quality",
    "severity": "critical",
+    "verification": {
+      "code_examined": "const result = await paymentGateway.charge(order.total, order.paymentMethod);",
+      "line_range_examined": [34, 34],
+      "verification_method": "direct_code_inspection"
+    },
+    "is_impact_finding": false,
+    "checked_for_handling_elsewhere": true,
    "suggested_fix": "Wrap in try/catch: try { await paymentGateway.charge(...) } catch (error) { logger.error('Payment failed', error); throw new PaymentError(error); }",
    "confidence": 95
  },
@@ -248,6 +418,13 @@ Provide findings in JSON format:
    "description": "This email validation regex is duplicated in 4 other files (user.ts, auth.ts, profile.ts, settings.ts). Changes to validation rules require updating all copies.",
    "category": "quality",
    "severity": "high",
+    "verification": {
+      "code_examined": "const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$/;",
+      "line_range_examined": [15, 15],
+      "verification_method": "cross_file_trace"
+    },
+    "is_impact_finding": false,
+    "checked_for_handling_elsewhere": false,
    "suggested_fix": "Extract to shared utility: export const isValidEmail = (email) => /regex/.test(email); and import where needed",
    "confidence": 90
  }
@@ -6,6 +6,82 @@ You are a focused security review agent. You have been spawned by the orchestrat

 Perform a thorough security review of the provided code changes, focusing ONLY on security vulnerabilities. Do not review code quality, style, or other non-security concerns.

+## Phase 1: Understand the PR Intent (BEFORE Looking for Issues)
+
+**MANDATORY** - Before searching for issues, understand what this PR is trying to accomplish.
+
+1. **Read the provided context**
+   - PR description: What does the author say this does?
+   - Changed files: What areas of code are affected?
+   - Commits: How did the PR evolve?
+
+2. **Identify the change type**
+   - Bug fix: Correcting broken behavior
+   - New feature: Adding new capability
+   - Refactor: Restructuring without behavior change
+   - Performance: Optimizing existing code
+   - Cleanup: Removing dead code or improving organization
+
+3. **State your understanding** (include in your analysis)
+   ```
+   PR INTENT: This PR [verb] [what] by [how].
+   RISK AREAS: [what could go wrong specific to this change type]
+   ```
+
+**Only AFTER completing Phase 1, proceed to looking for issues.**
+
+Why this matters: Understanding intent prevents flagging intentional design decisions as bugs.
+
+## TRIGGER-DRIVEN EXPLORATION (CHECK YOUR DELEGATION PROMPT)
+
+**FIRST**: Check if your delegation prompt contains a `TRIGGER:` instruction.
+
+- **If TRIGGER is present** → Exploration is **MANDATORY**, even if the diff looks correct
+- **If no TRIGGER** → Use your judgment to explore or not
+
+### How to Explore (Bounded)
+
+1. **Read the trigger** - What pattern did the orchestrator identify?
+2. **Form the specific question** - "Do callers validate input before passing it here?" (not "what do callers do?")
+3. **Use Grep** to find call sites of the changed function/method
+4. **Use Read** to examine 3-5 callers
+5. **Answer the question** - Yes (report issue) or No (move on)
+6. **Stop** - Do not explore callers of callers (depth > 1)
+
+### Security-Specific Trigger Questions
+
+| Trigger | Security Question to Answer |
+|---------|----------------------------|
+| **Output contract changed** | Does the new output expose sensitive data that was previously hidden? |
+| **Input contract changed** | Do callers now pass unvalidated input where validation was assumed? |
+| **Failure contract changed** | Does the new failure mode leak security information or bypass checks? |
+| **Side effect removed** | Was the removed effect a security control (logging, audit, cleanup)? |
+| **Auth/validation removed** | Do callers assume this function validates/authorizes? |
+
+### Example Exploration
+
+```
+TRIGGER: Failure contract changed (now throws instead of returning null)
+QUESTION: Do callers handle the new exception securely?
+
+1. Grep for "authenticateUser(" → found 5 call sites
+2. Read api/login.ts:34 → catches exception, logs full error to response → ISSUE (info leak)
+3. Read api/admin.ts:12 → catches exception, returns generic error → OK
+4. Read middleware/auth.ts:78 → no try/catch, exception propagates → ISSUE (500 with stack trace)
+5. STOP - Found 2 security issues
+
+FINDINGS:
+- api/login.ts:34 - Exception message leaked to client (information disclosure)
+- middleware/auth.ts:78 - Unhandled exception exposes stack trace in production
+```
+
+### When NO Trigger is Given
+
+If the orchestrator doesn't specify a trigger, use your judgment:
+- Focus on security issues in the changed code first
+- Only explore callers if you suspect a security boundary issue
+- Don't explore "just to be thorough"
+
 ## CRITICAL: PR Scope and Context

 ### What IS in scope (report these issues):
@@ -135,6 +211,92 @@ Before reporting ANY finding, you MUST:

 **Your evidence must prove the issue exists - not just that you suspect it.**

+## Evidence Requirements (MANDATORY)
+
+Every finding you report MUST include a `verification` object with ALL of these fields:
+
+### Required Fields
+
+**code_examined** (string, min 1 character)
+The **exact code snippet** you examined. Copy-paste directly from the file:
+```
+CORRECT: "cursor.execute(f'SELECT * FROM users WHERE id={user_id}')"
+WRONG:   "SQL query that uses string interpolation"
+```
+
+**line_range_examined** (array of 2 integers)
+The exact line numbers [start, end] where the issue exists:
+```
+CORRECT: [45, 47]
+WRONG:   [1, 100]  // Too broad - you didn't examine all 100 lines
+```
+
+**verification_method** (one of these exact values)
+How you verified the issue:
+- `"direct_code_inspection"` - Found the issue directly in the code at the location
+- `"cross_file_trace"` - Traced through imports/calls to confirm the issue
+- `"test_verification"` - Verified through examination of test code
+- `"dependency_analysis"` - Verified through analyzing dependencies
+
+### Conditional Fields
+
+**is_impact_finding** (boolean, default false)
+Set to `true` ONLY if this finding is about impact on OTHER files (not the changed file):
+```
+TRUE:  "This change in utils.ts breaks the caller in auth.ts"
+FALSE: "This code in utils.ts has a bug" (issue is in the changed file)
+```
+
+**checked_for_handling_elsewhere** (boolean, default false)
+For ANY "missing X" claim (missing validation, missing sanitization, missing auth check):
+- Set `true` ONLY if you used Grep/Read tools to verify X is not handled elsewhere
+- Set `false` if you didn't search other files
+- **When true, include the search in your description:**
+  - "Searched `Grep('sanitize|escape|validate', 'src/api/')` - no input validation found"
+  - "Checked middleware via `Grep('authMiddleware|requireAuth', '**/*.ts')` - endpoint unprotected"
+
+```
+TRUE:  "Searched for sanitization in this file and callers - none found"
+FALSE: "This input should be sanitized" (didn't verify it's missing)
+```
+
+**If you cannot provide real evidence, you do not have a verified finding - do not report it.**
+
+**Search Before Claiming Absence:** Never claim protection is "missing" without searching for it first. Validation may exist in middleware, callers, or framework-level code.
+
+## Valid Outputs
+
+Finding issues is NOT the goal. Accurate review is the goal.
+
+### Valid: No Significant Issues Found
+If the code is well-implemented, say so:
+```json
+{
+  "findings": [],
+  "summary": "Reviewed [files]. No security issues found. The implementation correctly [positive observation about the code]."
+}
+```
+
+### Valid: Only Low-Severity Suggestions
+Minor improvements that don't block merge:
+```json
+{
+  "findings": [
+    {"severity": "low", "title": "Consider extracting magic number to constant", ...}
+  ],
+  "summary": "Code is sound. One minor suggestion for readability."
+}
+```
+
+### INVALID: Forced Issues
+Do NOT report issues just to have something to say:
+- Theoretical edge cases without evidence they're reachable
+- Style preferences not backed by project conventions
+- "Could be improved" without concrete problem
+- Pre-existing issues not introduced by this PR
+
+**Reporting nothing is better than reporting noise.** False positives erode trust faster than false negatives.
+
 ## Code Patterns to Flag

 ### JavaScript/TypeScript
@@ -189,6 +351,13 @@ Provide findings in JSON format:
    "description": "User input from req.params.id is directly interpolated into SQL query without sanitization. An attacker could inject malicious SQL to extract sensitive data or modify the database.",
    "category": "security",
    "severity": "critical",
+    "verification": {
+      "code_examined": "const query = `SELECT * FROM users WHERE id = ${req.params.id}`;",
+      "line_range_examined": [45, 45],
+      "verification_method": "direct_code_inspection"
+    },
+    "is_impact_finding": false,
+    "checked_for_handling_elsewhere": false,
    "suggested_fix": "Use parameterized queries: db.query('SELECT * FROM users WHERE id = ?', [req.params.id])",
    "confidence": 95
  },
@@ -199,6 +368,13 @@ Provide findings in JSON format:
    "description": "API secret is hardcoded as a string literal. If this code is committed to version control, the secret is exposed to anyone with repository access.",
    "category": "security",
    "severity": "critical",
+    "verification": {
+      "code_examined": "const API_SECRET = 'sk-prod-abc123xyz789';",
+      "line_range_examined": [12, 12],
+      "verification_method": "direct_code_inspection"
+    },
+    "is_impact_finding": false,
+    "checked_for_handling_elsewhere": false,
    "suggested_fix": "Move secret to environment variable: const API_SECRET = process.env.API_SECRET",
    "confidence": 100
  }
@@ -1,5 +1,7 @@
 # Auto-Build Framework Dependencies
-claude-agent-sdk>=0.1.19
+# SDK 0.1.22+ required for custom subagent support (bundles CLI 2.1.19+)
+# Earlier versions bundled CLI 2.1.9 which didn't properly register --agents flag
+claude-agent-sdk>=0.1.22
 python-dotenv>=1.0.0

 # TOML parsing fallback for Python < 3.11
@@ -27,6 +27,7 @@ Usage:
 from __future__ import annotations

 import json
+import logging
 import os
 import subprocess
 import sys
@@ -36,6 +37,8 @@ from pathlib import Path

 from core.gh_executable import get_gh_executable

+logger = logging.getLogger(__name__)
+
 try:
    from .file_lock import FileLock, atomic_write
 except (ImportError, ValueError, SystemError):
@@ -374,7 +377,7 @@ class BotDetector:
        # Save state
        self.state.save(self.state_dir)

-        print(
+        logger.info(
            f"[BotDetector] Marked PR #{pr_number} as reviewed at {commit_sha[:8]} "
            f"({len(self.state.reviewed_commits[pr_key])} total commits reviewed)"
        )
@@ -1086,9 +1086,9 @@ class PRContextGatherer:
        path_obj = Path(file_path)
        stem = path_obj.stem  # e.g., 'helpers' from 'utils/helpers.ts'

-        # Skip if stem is too generic (would match too many files)
-        if stem in ["index", "main", "app", "utils", "helpers", "types", "constants"]:
-            return dependents
+        # NOTE: We no longer skip generic filenames like "utils", "types", "index".
+        # The LLM-driven exploration system decides what's relevant based on the PR context.
+        # Widely-used utilities are often the MOST important files to track dependents for.

        # Build regex patterns and file extensions based on file type
        pattern = None
@@ -1134,7 +1134,9 @@ class PRContextGatherer:
                    # Check if we've hit the file limit
                    if files_checked >= max_files_to_check:
                        safe_print(
-                            f"[Context] File limit reached finding dependents for {file_path}"
+                            f"[Context] File scan limit ({max_files_to_check}) reached for {file_path}. "
+                            f"Found {len(dependents)} dependents. "
+                            f"LLM agents can explore additional callers if needed via Read/Grep tools."
                        )
                        return dependents

@@ -367,12 +367,21 @@ class PRReviewFinding:
    validation_evidence: str | None = None  # Code snippet examined during validation
    validation_explanation: str | None = None  # Why finding was validated/dismissed

-    # Cross-validation and confidence routing fields
-    confidence: float = 0.5  # Confidence score (0.0-1.0), defaults to medium confidence
+    # Cross-validation fields
+    # NOTE: confidence field is DEPRECATED - we use evidence-based validation, not confidence scores
+    # The finding-validator determines validity by examining actual code, not by confidence thresholds
+    confidence: float = 0.5  # DEPRECATED: No longer used for filtering
    source_agents: list[str] = field(
        default_factory=list
    )  # Which agents reported this finding
-    cross_validated: bool = False  # Whether multiple agents agreed on this finding
+    cross_validated: bool = (
+        False  # Whether multiple agents agreed on this finding (signal, not filter)
+    )
+
+    # Impact finding flag - indicates this finding is about code OUTSIDE the PR's changed files
+    # (e.g., callers affected by contract changes). Used by _is_finding_in_scope() to allow
+    # findings about related files that aren't directly in the PR diff.
+    is_impact_finding: bool = False

    def to_dict(self) -> dict:
        return {
@@ -398,6 +407,8 @@ class PRReviewFinding:
            "confidence": self.confidence,
            "source_agents": self.source_agents,
            "cross_validated": self.cross_validated,
+            # Impact finding flag
+            "is_impact_finding": self.is_impact_finding,
        }

    @classmethod
@@ -425,6 +436,8 @@ class PRReviewFinding:
            confidence=data.get("confidence", 0.5),
            source_agents=data.get("source_agents", []),
            cross_validated=data.get("cross_validated", False),
+            # Impact finding flag
+            is_impact_finding=data.get("is_impact_finding", False),
        )


@@ -22,30 +22,6 @@ except (ImportError, ValueError, SystemError):
 class FindingValidator:
    """Validates and filters AI-generated PR review findings."""

-    # Vague patterns that indicate low-quality findings
-    VAGUE_PATTERNS = [
-        "could be improved",
-        "consider using",
-        "might want to",
-        "you may want",
-        "it would be better",
-        "possibly consider",
-        "perhaps use",
-        "potentially add",
-        "you should consider",
-        "it might be good",
-    ]
-
-    # Generic suggestions without specifics
-    GENERIC_PATTERNS = [
-        "improve this",
-        "fix this",
-        "change this",
-        "update this",
-        "refactor this",
-        "review this",
-    ]
-
    # Minimum lengths for quality checks
    MIN_DESCRIPTION_LENGTH = 30
    MIN_SUGGESTED_FIX_LENGTH = 20
@@ -123,10 +99,6 @@ class FindingValidator:
            # Update the finding with corrected line
            finding.line = corrected.line

-        # Check for false positives
-        if self._is_false_positive(finding):
-            return False
-
        # Check confidence threshold
        if not self._meets_confidence_threshold(finding):
            return False
@@ -294,51 +266,6 @@ class FindingValidator:

        return finding

-    def _is_false_positive(self, finding: PRReviewFinding) -> bool:
-        """
-        Detect likely false positives.
-
-        Args:
-            finding: Finding to check
-
-        Returns:
-            True if likely a false positive, False otherwise
-        """
-        description_lower = finding.description.lower()
-
-        # Check for vague descriptions
-        for pattern in self.VAGUE_PATTERNS:
-            if pattern in description_lower:
-                # Vague low/medium findings are likely FPs
-                if finding.severity in [ReviewSeverity.LOW, ReviewSeverity.MEDIUM]:
-                    return True
-
-        # Check for generic suggestions
-        for pattern in self.GENERIC_PATTERNS:
-            if pattern in description_lower:
-                if finding.severity == ReviewSeverity.LOW:
-                    return True
-
-        # Check for generic suggestions without specifics
-        if (
-            not finding.suggested_fix
-            or len(finding.suggested_fix) < self.MIN_SUGGESTED_FIX_LENGTH
-        ):
-            if finding.severity == ReviewSeverity.LOW:
-                return True
-
-        # Check for style findings without clear justification
-        if finding.category.value == "style":
-            # Style findings should have good suggestions
-            if not finding.suggested_fix or len(finding.suggested_fix) < 30:
-                return True
-
-        # Check for overly short descriptions
-        if len(finding.description) < 50 and finding.severity == ReviewSeverity.LOW:
-            return True
-
-        return False
-
    def _score_actionability(self, finding: PRReviewFinding) -> float:
        """
        Score how actionable a finding is (0.0 to 1.0).
@@ -0,0 +1,33 @@
+"""
+Agent Utilities
+===============
+
+Shared utility functions for GitHub PR review agents.
+"""
+
+from pathlib import Path
+
+
+def create_working_dir_injector(working_dir: Path):
+    """Factory that creates a prompt injector with working directory context.
+
+    Args:
+        working_dir: The working directory path to inject into prompts
+
+    Returns:
+        A function that takes (prompt, fallback) and returns the prompt with
+        working directory prefix prepended.
+    """
+    working_dir_prefix = (
+        f"## Working Directory\n\n"
+        f"Your working directory is: `{working_dir.resolve()}`\n"
+        f"All file paths should be relative to this directory.\n"
+        f"Use the Read, Grep, and Glob tools to examine files.\n\n"
+    )
+
+    def with_working_dir(prompt: str | None, fallback: str) -> str:
+        """Inject working directory context into agent prompt."""
+        base = prompt or fallback
+        return f"{working_dir_prefix}{base}"
+
+    return with_working_dir
@@ -43,6 +43,7 @@ try:
        PRReviewResult,
        ReviewSeverity,
    )
+    from .agent_utils import create_working_dir_injector
    from .category_utils import map_category
    from .io_utils import safe_print
    from .pr_worktree_manager import PRWorktreeManager
@@ -62,6 +63,7 @@ except (ImportError, ValueError, SystemError):
        ReviewSeverity,
    )
    from phase_config import get_thinking_budget, resolve_model_id
+    from services.agent_utils import create_working_dir_injector
    from services.category_utils import map_category
    from services.io_utils import safe_print
    from services.pr_worktree_manager import PRWorktreeManager
@@ -183,22 +185,35 @@ class ParallelFollowupReviewer:
        """
        self.worktree_manager.remove_worktree(worktree_path)

-    def _define_specialist_agents(self) -> dict[str, AgentDefinition]:
+    def _define_specialist_agents(
+        self, project_root: Path | None = None
+    ) -> dict[str, AgentDefinition]:
        """
        Define specialist agents for follow-up review.

        Each agent has:
        - description: When the orchestrator should invoke this agent
-        - prompt: System prompt for the agent
+        - prompt: System prompt for the agent (includes working directory)
        - tools: Tools the agent can use (read-only for PR review)
        - model: "inherit" = use same model as orchestrator (user's choice)
+
+        Args:
+            project_root: Working directory for the agents (worktree path).
+                         If None, falls back to self.project_dir.
        """
+        # Use provided project_root or fall back to default
+        working_dir = project_root or self.project_dir
+
        # Load agent prompts from files
        resolution_prompt = self._load_prompt("pr_followup_resolution_agent.md")
        newcode_prompt = self._load_prompt("pr_followup_newcode_agent.md")
        comment_prompt = self._load_prompt("pr_followup_comment_agent.md")
        validator_prompt = self._load_prompt("pr_finding_validator.md")

+        # CRITICAL: Inject working directory into all prompts
+        # Subagents don't inherit cwd from parent, so they need explicit path info
+        with_working_dir = create_working_dir_injector(working_dir)
+
        return {
            "resolution-verifier": AgentDefinition(
                description=(
@@ -207,8 +222,10 @@ class ParallelFollowupReviewer:
                    "are truly fixed, partially fixed, or still unresolved. "
                    "Invoke when: There are previous findings to verify."
                ),
-                prompt=resolution_prompt
-                or "You verify whether previous findings are resolved.",
+                prompt=with_working_dir(
+                    resolution_prompt,
+                    "You verify whether previous findings are resolved.",
+                ),
                tools=["Read", "Grep", "Glob"],
                model="inherit",
            ),
@@ -219,7 +236,9 @@ class ParallelFollowupReviewer:
                    "Invoke when: There are substantial code changes (>50 lines diff) or "
                    "changes to security-sensitive areas."
                ),
-                prompt=newcode_prompt or "You review new code for issues.",
+                prompt=with_working_dir(
+                    newcode_prompt, "You review new code for issues."
+                ),
                tools=["Read", "Grep", "Glob"],
                model="inherit",
            ),
@@ -230,7 +249,9 @@ class ParallelFollowupReviewer:
                    "unanswered questions and valid concerns. "
                    "Invoke when: There are comments or formal reviews since last review."
                ),
-                prompt=comment_prompt or "You analyze comments and feedback.",
+                prompt=with_working_dir(
+                    comment_prompt, "You analyze comments and feedback."
+                ),
                tools=["Read", "Grep", "Glob"],
                model="inherit",
            ),
@@ -243,8 +264,10 @@ class ParallelFollowupReviewer:
                    "CRITICAL: Invoke for ALL unresolved findings after resolution-verifier runs. "
                    "Invoke when: There are findings marked as unresolved that need validation."
                ),
-                prompt=validator_prompt
-                or "You validate whether unresolved findings are real issues.",
+                prompt=with_working_dir(
+                    validator_prompt,
+                    "You validate whether unresolved findings are real issues.",
+                ),
                tools=["Read", "Grep", "Glob"],
                model="inherit",
            ),
@@ -487,6 +510,9 @@ The SDK will run invoked agents in parallel automatically.
                    "using local checkout"
                )

+            # Capture agent definitions for debug logging (AFTER worktree creation)
+            agent_defs = self._define_specialist_agents(project_root)
+
            # Use model and thinking level from config (user settings)
            # Resolve model shorthand via environment variable override if configured
            model_shorthand = self.config.model or "sonnet"
@@ -499,14 +525,14 @@ The SDK will run invoked agents in parallel automatically.
                f"thinking_level={thinking_level}, thinking_budget={thinking_budget}"
            )

-            # Create client with subagents defined
+            # Create client with subagents defined (using worktree path)
            client = create_client(
                project_dir=project_root,
                spec_dir=self.github_dir,
                model=model,
                agent_type="pr_followup_parallel",
                max_thinking_tokens=thinking_budget,
-                agents=self._define_specialist_agents(),
+                agents=self._define_specialist_agents(project_root),
                output_format={
                    "type": "json_schema",
                    "schema": ParallelFollowupResponse.model_json_schema(),
@@ -534,6 +560,8 @@ The SDK will run invoked agents in parallel automatically.
                    client=client,
                    context_name="ParallelFollowup",
                    model=model,
+                    system_prompt=prompt,
+                    agent_definitions=agent_defs,
                )

                # Check for stream processing errors
@@ -883,6 +911,7 @@ The SDK will run invoked agents in parallel automatically.
                                    validation_status=validation_status,
                                    validation_evidence=validation_evidence,
                                    validation_explanation=validation_explanation,
+                                    is_impact_finding=original.is_impact_finding,
                                )
                            )

@@ -903,6 +932,7 @@ The SDK will run invoked agents in parallel automatically.
                        line=nf.line,
                        suggested_fix=nf.suggested_fix,
                        fixable=nf.fixable,
+                        is_impact_finding=getattr(nf, "is_impact_finding", False),
                    )
                )

@@ -28,6 +28,50 @@ from typing import Literal

 from pydantic import BaseModel, Field

+# =============================================================================
+# Verification Evidence (Required for All Findings)
+# =============================================================================
+
+
+class VerificationEvidence(BaseModel):
+    """Evidence that a finding was verified against actual code.
+
+    All fields are required - schema enforcement guarantees evidence exists.
+    This shifts quality control from programmatic filters to schema enforcement.
+    """
+
+    code_examined: str = Field(
+        min_length=1,
+        description=(
+            "REQUIRED: Exact code snippet that was examined. "
+            "Must be actual code from the file, not a description of code. "
+            "Copy-paste the relevant lines directly."
+        ),
+    )
+    line_range_examined: list[int] = Field(
+        min_length=2,
+        max_length=2,
+        description=(
+            "Start and end line numbers [start, end] of the examined code. "
+            "Must match the code in code_examined."
+        ),
+    )
+    verification_method: Literal[
+        "direct_code_inspection",
+        "cross_file_trace",
+        "test_verification",
+        "dependency_analysis",
+    ] = Field(
+        description=(
+            "How the issue was verified: "
+            "direct_code_inspection = found issue directly in the code shown; "
+            "cross_file_trace = traced through imports/calls to find the issue; "
+            "test_verification = verified through examination of test code; "
+            "dependency_analysis = verified through analyzing dependencies"
+        )
+    )
+
+
 # =============================================================================
 # Common Finding Types
 # =============================================================================
@@ -48,7 +92,10 @@ class BaseFinding(BaseModel):
    fixable: bool = Field(False, description="Whether this can be auto-fixed")
    evidence: str | None = Field(
        None,
-        description="Actual code snippet proving the issue exists. Required for validation.",
+        description="DEPRECATED: Use verification.code_examined instead. Will be removed in Phase 5.",
+    )
+    verification: VerificationEvidence = Field(
+        description="Evidence that this finding was verified against actual code"
    )


@@ -155,6 +202,9 @@ class FollowupFinding(BaseModel):
    line: int = Field(0, description="Line number of the issue")
    suggested_fix: str | None = Field(None, description="How to fix this issue")
    fixable: bool = Field(False, description="Whether this can be auto-fixed")
+    verification: VerificationEvidence = Field(
+        description="Evidence that this finding was verified against actual code"
+    )


 class FollowupReviewResponse(BaseModel):
@@ -323,7 +373,10 @@ class OrchestratorFinding(BaseModel):
    suggestion: str | None = Field(None, description="How to fix this issue")
    evidence: str | None = Field(
        None,
-        description="Actual code snippet proving the issue exists. Required for validation.",
+        description="DEPRECATED: Use verification.code_examined instead. Will be removed in Phase 5.",
+    )
+    verification: VerificationEvidence = Field(
+        description="Evidence that this finding was verified against actual code"
    )


@@ -399,7 +452,25 @@ class ParallelOrchestratorFinding(BaseModel):
    )
    evidence: str | None = Field(
        None,
-        description="Actual code snippet proving the issue exists. Required for validation.",
+        description="DEPRECATED: Use verification.code_examined instead. Will be removed in Phase 5.",
+    )
+    verification: VerificationEvidence = Field(
+        description="Evidence that this finding was verified against actual code"
+    )
+    is_impact_finding: bool = Field(
+        False,
+        description=(
+            "True if this finding is about impact on OTHER files (not the changed file). "
+            "Impact findings may reference files outside the PR's changed files list."
+        ),
+    )
+    checked_for_handling_elsewhere: bool = Field(
+        False,
+        description=(
+            "For 'missing X' claims (missing error handling, missing validation, etc.), "
+            "True if the agent verified X is not handled elsewhere in the codebase. "
+            "False if this is a 'missing X' claim but other locations were not checked."
+        ),
    )
    suggested_fix: str | None = Field(None, description="How to fix this issue")
    fixable: bool = Field(False, description="Whether this can be auto-fixed")
@@ -428,6 +499,44 @@ class AgentAgreement(BaseModel):
    )


+class DismissedFinding(BaseModel):
+    """A finding that was validated and dismissed as a false positive.
+
+    Included in output for transparency - users can see what was investigated and why it was dismissed.
+    """
+
+    id: str = Field(description="Original finding ID")
+    original_title: str = Field(description="Original finding title")
+    original_severity: Literal["critical", "high", "medium", "low"] = Field(
+        description="Original severity assigned by specialist"
+    )
+    original_file: str = Field(description="File where issue was claimed")
+    original_line: int = Field(0, description="Line where issue was claimed")
+    dismissal_reason: str = Field(
+        description="Why this finding was dismissed as a false positive"
+    )
+    validation_evidence: str = Field(
+        description="Actual code examined that disproved the finding"
+    )
+
+
+class ValidationSummary(BaseModel):
+    """Summary of validation results for transparency."""
+
+    total_findings_from_specialists: int = Field(
+        description="Total findings reported by all specialist agents"
+    )
+    confirmed_valid: int = Field(
+        description="Findings confirmed as real issues by validator"
+    )
+    dismissed_false_positive: int = Field(
+        description="Findings dismissed as false positives by validator"
+    )
+    needs_human_review: int = Field(
+        0, description="Findings that couldn't be definitively validated"
+    )
+
+
 class ParallelOrchestratorResponse(BaseModel):
    """Complete response schema for parallel orchestrator PR review."""

@@ -438,8 +547,20 @@ class ParallelOrchestratorResponse(BaseModel):
        default_factory=list,
        description="List of agent names that were invoked",
    )
+    validation_summary: ValidationSummary | None = Field(
+        None,
+        description="Summary of validation results (total, confirmed, dismissed, needs_review)",
+    )
    findings: list[ParallelOrchestratorFinding] = Field(
-        default_factory=list, description="All findings from synthesis"
+        default_factory=list,
+        description="Validated findings only (confirmed_valid or needs_human_review)",
+    )
+    dismissed_findings: list[DismissedFinding] = Field(
+        default_factory=list,
+        description=(
+            "Findings that were validated and dismissed as false positives. "
+            "Included for transparency - users can see what was investigated."
+        ),
    )
    agent_agreement: AgentAgreement = Field(
        default_factory=AgentAgreement,
@@ -495,7 +616,10 @@ class ParallelFollowupFinding(BaseModel):
    )
    evidence: str | None = Field(
        None,
-        description="Actual code snippet proving the issue exists. Required for validation.",
+        description="DEPRECATED: Use verification.code_examined instead. Will be removed in Phase 5.",
+    )
+    verification: VerificationEvidence = Field(
+        description="Evidence that this finding was verified against actual code"
    )
    suggested_fix: str | None = Field(None, description="How to fix this issue")
    fixable: bool = Field(False, description="Whether this can be auto-fixed")
@@ -505,6 +629,14 @@ class ParallelFollowupFinding(BaseModel):
    related_to_previous: str | None = Field(
        None, description="ID of related previous finding if this is a regression"
    )
+    is_impact_finding: bool = Field(
+        False,
+        description=(
+            "True if this finding is about impact on OTHER files (callers, dependents) "
+            "outside the PR's changed files. Used by _is_finding_in_scope() to allow "
+            "findings about related files that aren't directly in the PR diff."
+        ),
+    )


 class CommentAnalysis(BaseModel):
@@ -594,7 +594,7 @@ Focus on:
 - Insecure cryptography
 - Input validation issues

-Output findings in JSON format with high confidence (>80%) only.
+Output findings in JSON format with evidence from the actual code.
 """


@@ -611,5 +611,5 @@ Focus on:
 - Pattern adherence
 - Maintainability

-Output findings in JSON format with high confidence (>80%) only.
+Output findings in JSON format with evidence from the actual code.
 """
@@ -25,6 +25,243 @@ logger = logging.getLogger(__name__)
 # Check if debug mode is enabled
 DEBUG_MODE = os.environ.get("DEBUG", "").lower() in ("true", "1", "yes")

+# ── TEMPORARY: Per-PR full agent communication logger (v2) ────────────
+# Writes every message to .auto-claude/github/pr/debug_logs/<context>_<ts>.log
+# Remove after measurement phase is complete.
+import datetime as _dt
+import json as _json
+from pathlib import Path as _Path
+
+# Derive project root dynamically from this file's location
+# sdk_utils.py is at: apps/backend/runners/github/services/sdk_utils.py
+# So project root is 5 levels up
+_PROJECT_ROOT = _Path(__file__).resolve().parent.parent.parent.parent.parent
+_PR_LOG_DIR = _PROJECT_ROOT / ".auto-claude" / "github" / "pr" / "debug_logs"
+
+
+class _PRDebugLogger:
+    """Writes full agent communication to a log file for review.
+
+    Improvements (v2):
+    - System prompt and agent definitions logged at session start
+    - No truncation on thinking, text, tool input, or tool results
+    - No duplicate logging (single structured dump per message)
+    - Empty/whitespace content shown via repr()
+    - Agent attribution via subagent_tool_ids mapping
+    """
+
+    def __init__(self, context_name: str, model: str | None = None):
+        self._f = None
+        self._subagent_tool_ids: dict[str, str] = {}  # tool_id -> agent_name
+
+        try:
+            _PR_LOG_DIR.mkdir(parents=True, exist_ok=True)
+            ts = _dt.datetime.now().strftime("%Y%m%d_%H%M%S")
+            self.path = _PR_LOG_DIR / f"{context_name}_{ts}.log"
+            self._f = open(self.path, "w", encoding="utf-8")
+            self._write(
+                f"=== {context_name} Session Started at {_dt.datetime.now().isoformat()} ==="
+            )
+            if model:
+                self._write(f"Model: {model}")
+            self._write("")
+        except OSError as e:
+            # Failed to create directory or open file - logging disabled
+            logger.warning(f"PR debug logger disabled: {e}")
+            self.path = None
+
+    def _write(self, text: str):
+        # Skip logging if file handle was not created successfully
+        if self._f is None:
+            return
+        try:
+            self._f.write(text + "\n")
+            self._f.flush()
+        except (OSError, ValueError) as e:
+            # File write failed (file closed, disk full, etc.) - disable logging
+            logger.warning(f"PR debug logger write failed: {e}")
+            self._f = None
+
+    # ── Session preamble loggers ──────────────────────────────────────
+
+    def log_system_prompt(self, prompt: str):
+        """Log the full system prompt (no truncation)."""
+        self._write(f"\n{'#' * 80}")
+        self._write("# SYSTEM PROMPT (full orchestrator instructions + PR context)")
+        self._write(f"# Length: {len(prompt)} chars")
+        self._write(f"{'#' * 80}")
+        self._write(prompt)
+        self._write(f"{'#' * 80}\n")
+
+    def log_agent_definitions(self, agents: dict):
+        """Log all specialist agent definitions (prompts, tools, descriptions)."""
+        self._write(f"\n{'#' * 80}")
+        self._write(f"# AGENT DEFINITIONS ({len(agents)} specialists)")
+        self._write(f"{'#' * 80}")
+        for name, defn in agents.items():
+            self._write(f"\n--- Agent: {name} ---")
+            self._write(f"  description: {getattr(defn, 'description', 'N/A')}")
+            self._write(f"  model: {getattr(defn, 'model', 'N/A')}")
+            self._write(f"  tools: {getattr(defn, 'tools', 'N/A')}")
+            prompt = getattr(defn, "prompt", "")
+            self._write(f"  prompt ({len(prompt)} chars):")
+            self._write(prompt)
+        self._write(f"{'#' * 80}\n")
+
+    # ── Agent attribution ─────────────────────────────────────────────
+
+    def set_subagent_mapping(self, mapping: dict[str, str]):
+        """Update the tool_id -> agent_name mapping for attribution."""
+        self._subagent_tool_ids = mapping
+
+    def _get_agent_label(self, tool_id: str) -> str:
+        """Return agent label if this tool_id belongs to a known subagent."""
+        agent = self._subagent_tool_ids.get(tool_id)
+        return f" [Agent:{agent}]" if agent else ""
+
+    # ── Per-message logger (single structured dump) ───────────────────
+
+    def log_message(self, msg_count: int, msg_type: str, msg: object):
+        self._write(f"\n{'=' * 80}")
+        self._write(f"--- Message #{msg_count} [{msg_type}] ---")
+        self._write(f"{'=' * 80}")
+        self._dump_raw(msg)
+
+    def _dump_raw(self, msg: object, indent: int = 0):
+        """Dump full raw message content recursively — NO truncation."""
+        prefix = "  " * indent
+        # Content blocks
+        if hasattr(msg, "content"):
+            content = msg.content
+            if isinstance(content, list):
+                self._write(f"{prefix}[content] ({len(content)} blocks):")
+                for i, block in enumerate(content):
+                    block_type = type(block).__name__
+                    self._write(f"{prefix}  [{i}] {block_type}:")
+                    self._dump_block(block, indent + 2)
+            elif isinstance(content, str):
+                if not content or content.isspace():
+                    self._write(
+                        f"{prefix}[content] (string, {len(content)} chars): {repr(content)}"
+                    )
+                else:
+                    self._write(f"{prefix}[content] (string, {len(content)} chars):")
+                    self._write(content)
+            else:
+                self._write(f"{prefix}[content] ({type(content).__name__}):")
+                self._write(f"{prefix}  {str(content)}")
+
+        # Role / type
+        if hasattr(msg, "role"):
+            self._write(f"{prefix}[role] {msg.role}")
+        if hasattr(msg, "type") and not hasattr(msg, "content"):
+            self._write(f"{prefix}[type] {msg.type}")
+
+        # Structured output
+        if hasattr(msg, "structured_output") and msg.structured_output:
+            self._write(f"{prefix}[structured_output]:")
+            try:
+                self._write(_json.dumps(msg.structured_output, indent=2, default=str))
+            except Exception:
+                self._write(f"{prefix}  {str(msg.structured_output)}")
+
+        # Result message fields
+        if hasattr(msg, "subtype"):
+            self._write(f"{prefix}[subtype] {msg.subtype}")
+        if hasattr(msg, "is_error"):
+            self._write(f"{prefix}[is_error] {msg.is_error}")
+        if hasattr(msg, "duration_ms"):
+            self._write(f"{prefix}[duration_ms] {msg.duration_ms}")
+        if hasattr(msg, "session_id"):
+            self._write(f"{prefix}[session_id] {msg.session_id}")
+
+        # Catch-all for messages without content blocks
+        for attr in ("text", "thinking", "name", "id", "input", "tool_use_id"):
+            if hasattr(msg, attr) and not hasattr(msg, "content"):
+                val = getattr(msg, attr)
+                if val is not None:
+                    self._write(f"{prefix}[{attr}] {str(val)}")
+
+    def _dump_block(self, block: object, indent: int = 0):
+        """Dump a single content block — NO truncation."""
+        prefix = "  " * indent
+        block_type = getattr(block, "type", type(block).__name__)
+
+        if block_type in ("text", "TextBlock") and hasattr(block, "text"):
+            text = block.text
+            if not text or text.isspace():
+                self._write(f"{prefix}[text] ({len(text)} chars): {repr(text)}")
+            else:
+                self._write(f"{prefix}[text] ({len(text)} chars):")
+                self._write(text)
+
+        elif block_type in ("thinking", "ThinkingBlock") and hasattr(block, "thinking"):
+            text = block.thinking or getattr(block, "text", "")
+            self._write(f"{prefix}[thinking] ({len(text)} chars):")
+            self._write(text)
+
+        elif block_type in ("tool_use", "ToolUseBlock"):
+            tool_name = getattr(block, "name", "unknown")
+            tool_id = getattr(block, "id", "unknown")
+            tool_input = getattr(block, "input", {})
+            agent_label = self._get_agent_label(tool_id)
+            self._write(f"{prefix}[tool_use] {tool_name} (id={tool_id}){agent_label}")
+            try:
+                self._write(_json.dumps(tool_input, indent=2, default=str))
+            except Exception:
+                self._write(str(tool_input))
+
+        elif block_type in ("tool_result", "ToolResultBlock"):
+            tool_id = getattr(block, "tool_use_id", "unknown")
+            is_error = getattr(block, "is_error", False)
+            result = getattr(block, "content", "")
+            if isinstance(result, list):
+                result = " ".join(str(getattr(c, "text", c)) for c in result)
+            status = "ERROR" if is_error else "OK"
+            agent_label = self._get_agent_label(tool_id)
+            self._write(
+                f"{prefix}[tool_result] (tool_id={tool_id}) {status}{agent_label}"
+            )
+            self._write(str(result))
+
+        else:
+            # Unknown block type — dump everything we can
+            self._write(f"{prefix}[{block_type}] (raw dump):")
+            for attr in dir(block):
+                if not attr.startswith("_"):
+                    try:
+                        val = getattr(block, attr)
+                        if not callable(val):
+                            self._write(f"{prefix}  {attr}: {str(val)}")
+                    except Exception:
+                        pass
+
+    # ── Structured output (standalone, for final result) ──────────────
+
+    def log_structured_output(self, output: dict):
+        self._write("[STRUCTURED_OUTPUT]")
+        try:
+            self._write(_json.dumps(output, indent=2, default=str))
+        except Exception:
+            self._write(str(output))
+
+    # ── Session close ─────────────────────────────────────────────────
+
+    def close(self, summary: dict):
+        self._write("\n=== Session Ended ===")
+        self._write(f"Messages: {summary.get('msg_count', '?')}")
+        self._write(f"Agents invoked: {summary.get('agents_invoked', [])}")
+        self._write(f"Error: {summary.get('error')}")
+        self._write(f"Log file: {self.path}")
+        if self._f is not None:
+            try:
+                self._f.close()
+            except OSError as e:
+                logger.warning(f"PR debug logger close failed: {e}")
+
+
+# ── END TEMPORARY ──────────────────────────────────────────────────────
+

 def _short_model_name(model: str | None) -> str:
    """Convert full model name to a short display name for logs.
@@ -133,6 +370,8 @@ async def process_sdk_stream(
    on_structured_output: Callable[[dict[str, Any]], None] | None = None,
    context_name: str = "SDK",
    model: str | None = None,
+    system_prompt: str | None = None,
+    agent_definitions: dict | None = None,
 ) -> dict[str, Any]:
    """
    Process SDK response stream with customizable callbacks.
@@ -153,6 +392,8 @@ async def process_sdk_stream(
        on_structured_output: Callback for structured output - receives dict
        context_name: Name for logging (e.g., "ParallelOrchestrator", "ParallelFollowup")
        model: Model name for logging (e.g., "claude-sonnet-4-5-20250929")
+        system_prompt: Full system prompt sent to the agent (logged at session start)
+        agent_definitions: Dict of agent name -> AgentDefinition (logged at session start)

    Returns:
        Dictionary with:
@@ -172,6 +413,15 @@ async def process_sdk_stream(
    subagent_tool_ids: dict[str, str] = {}  # tool_id -> agent_name
    completed_agent_tool_ids: set[str] = set()  # tool_ids of completed agents

+    # TEMPORARY: per-PR debug file logger
+    _dbg = _PRDebugLogger(context_name, model=model)
+
+    # Log session preamble: system prompt and agent definitions
+    if system_prompt:
+        _dbg.log_system_prompt(system_prompt)
+    if agent_definitions:
+        _dbg.log_agent_definitions(agent_definitions)
+
    safe_print(f"[{context_name}] Processing SDK stream...")
    if DEBUG_MODE:
        safe_print(f"[DEBUG {context_name}] Awaiting response stream...")
@@ -185,6 +435,7 @@ async def process_sdk_stream(
            try:
                msg_type = type(msg).__name__
                msg_count += 1
+                _dbg.log_message(msg_count, msg_type, msg)

                # Log progress periodically so user knows AI is working
                if msg_count - last_progress_log >= PROGRESS_LOG_INTERVAL:
@@ -253,11 +504,22 @@ async def process_sdk_stream(
                        agents_invoked.append(agent_name)
                        # Track this tool ID to log its result later
                        subagent_tool_ids[tool_id] = agent_name
+                        _dbg.set_subagent_mapping(subagent_tool_ids)
                        # Log with model info if available
                        model_info = f" [{_short_model_name(model)}]" if model else ""
                        safe_print(
                            f"[{context_name}] Invoking agent: {agent_name}{model_info}"
                        )
+                        # Log delegation prompt for debugging trigger system
+                        delegation_prompt = tool_input.get("prompt", "")
+                        if delegation_prompt:
+                            # Show first 300 chars of delegation prompt
+                            prompt_preview = delegation_prompt[:300]
+                            if len(delegation_prompt) > 300:
+                                prompt_preview += "..."
+                            safe_print(
+                                f"[{context_name}] Delegation prompt for {agent_name}: {prompt_preview}"
+                            )
                    elif tool_name != "StructuredOutput":
                        # Log meaningful tool info (not just tool name)
                        tool_detail = _get_tool_detail(tool_name, tool_input)
@@ -393,6 +655,7 @@ async def process_sdk_stream(
                    # Only capture if we don't already have it (avoid duplicates)
                    if structured_output is None:
                        structured_output = msg.structured_output
+                        _dbg.log_structured_output(msg.structured_output)
                        safe_print(f"[{context_name}] Received structured output")
                        if on_structured_output:
                            on_structured_output(msg.structured_output)
@@ -465,7 +728,7 @@ async def process_sdk_stream(

    safe_print(f"[{context_name}] Session ended. Total messages: {msg_count}")

-    return {
+    result = {
        "result_text": result_text,
        "structured_output": structured_output,
        "agents_invoked": agents_invoked,
@@ -473,3 +736,6 @@ async def process_sdk_stream(
        "subagent_tool_ids": subagent_tool_ids,
        "error": stream_error,
    }
+    _dbg.close(result)
+    safe_print(f"[{context_name}] Full debug log: {_dbg.path}")
+    return result
@@ -99,7 +99,7 @@ describe('Rate Limit Detector', () => {
    it('should return false for non-rate-limit errors', async () => {
      const { isRateLimitError } = await import('../rate-limit-detector');

-      expect(isRateLimitError('authentication required')).toBe(false);
+      expect(isRateLimitError('[CLI] authentication required')).toBe(false);
      expect(isRateLimitError('Task completed')).toBe(false);
    });
  });
@@ -135,10 +135,10 @@ describe('Auth Failure Detection', () => {
  });

  describe('detectAuthFailure', () => {
-    it('should detect "authentication required" pattern', async () => {
+    it('should detect "authentication required" pattern with bracket prefix', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Error: authentication required';
+      const output = '[CLI] authentication required';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -146,40 +146,40 @@ describe('Auth Failure Detection', () => {
      expect(result.message).toContain('authentication required');
    });

-    it('should detect "authentication is required" pattern', async () => {
+    it('should detect "authentication is required" pattern with bracket prefix', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Authentication is required to proceed';
+      const output = '[Auth] Authentication is required to proceed';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
      expect(result.failureType).toBe('missing');
    });

-    it('should detect "not authenticated" pattern', async () => {
+    it('should detect "not authenticated" pattern with bracket prefix', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Error: not authenticated';
+      const output = '[Error] not authenticated';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
      expect(result.failureType).toBe('missing');
    });

-    it('should detect "not yet authenticated" pattern', async () => {
+    it('should detect "not yet authenticated" pattern with bracket prefix', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'You are not yet authenticated';
+      const output = '[CLI] not yet authenticated';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
      expect(result.failureType).toBe('missing');
    });

-    it('should detect "login required" pattern', async () => {
+    it('should detect "login required" pattern with bracket prefix', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Login required';
+      const output = '[CLI] Login required';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -189,7 +189,7 @@ describe('Auth Failure Detection', () => {
    it('should detect "oauth token invalid" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'OAuth token is invalid';
+      const output = 'Error: invalid token';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -199,7 +199,7 @@ describe('Auth Failure Detection', () => {
    it('should detect "oauth token expired" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'OAuth token expired';
+      const output = 'OAuth token has expired';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -209,7 +209,7 @@ describe('Auth Failure Detection', () => {
    it('should detect "oauth token missing" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'OAuth token missing';
+      const output = '[CLI] authentication required - OAuth token missing';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -219,39 +219,39 @@ describe('Auth Failure Detection', () => {
    it('should detect "unauthorized" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Error: Unauthorized';
+      const output = 'Error: unauthorized access';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
      expect(result.failureType).toBe('invalid');
    });

-    it('should detect "please log in" pattern', async () => {
+    it('should detect "please log in" pattern with CLI format', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Please log in to continue';
+      const output = '· Please run /login to continue';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
-      // "please log in" doesn't contain 'required' keyword, so classified as 'unknown'
+      // Contains 'login' pattern
      expect(result.failureType).toBeDefined();
    });

-    it('should detect "please authenticate" pattern', async () => {
+    it('should detect "please authenticate" pattern with Error prefix', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Please authenticate before proceeding';
+      const output = 'Error: authentication required before proceeding';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
-      // "please authenticate" doesn't contain 'required' keyword, so classified as 'unknown'
+      // "Error: ... authentication" matches the pattern
      expect(result.failureType).toBeDefined();
    });

    it('should detect "invalid credentials" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Invalid credentials provided';
+      const output = 'Error: invalid token credentials provided';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -261,26 +261,26 @@ describe('Auth Failure Detection', () => {
    it('should detect "invalid token" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Invalid token';
+      const output = 'Error: invalid token';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
      expect(result.failureType).toBe('invalid');
    });

-    it('should detect "auth failed" pattern', async () => {
+    it('should detect "auth failed" pattern with authentication_error type', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Auth failed';
+      const output = '{"type":"authentication_error","message":"Auth failed"}';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
    });

-    it('should detect "authentication error" pattern', async () => {
+    it('should detect "authentication error" pattern with JSON type', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Authentication error occurred';
+      const output = '{"type": "authentication_error", "message": "Authentication error occurred"}';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -289,7 +289,7 @@ describe('Auth Failure Detection', () => {
    it('should detect "session expired" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Your session expired';
+      const output = 'Please obtain a new token - your session expired';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -299,7 +299,7 @@ describe('Auth Failure Detection', () => {
    it('should detect "access denied" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Access denied';
+      const output = 'status: 401 - Access denied';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -309,7 +309,7 @@ describe('Auth Failure Detection', () => {
    it('should detect "permission denied" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Permission denied';
+      const output = 'HTTP 401 - Permission denied';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -329,7 +329,7 @@ describe('Auth Failure Detection', () => {
    it('should detect "credentials missing" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Credentials are missing';
+      const output = '[CLI] authentication required - credentials are missing';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -339,7 +339,7 @@ describe('Auth Failure Detection', () => {
    it('should detect "credentials expired" pattern', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = 'Credentials expired';
+      const output = 'Please refresh your existing token - credentials expired';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -375,7 +375,7 @@ describe('Auth Failure Detection', () => {
    it('should include profile ID in result', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const result = detectAuthFailure('authentication required', 'custom-profile');
+      const result = detectAuthFailure('[CLI] authentication required', 'custom-profile');

      expect(result.isAuthFailure).toBe(true);
      expect(result.profileId).toBe('custom-profile');
@@ -384,7 +384,7 @@ describe('Auth Failure Detection', () => {
    it('should use active profile ID when not specified', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const result = detectAuthFailure('authentication required');
+      const result = detectAuthFailure('[Auth] authentication required');

      expect(result.isAuthFailure).toBe(true);
      expect(result.profileId).toBe('test-profile-id');
@@ -403,7 +403,7 @@ describe('Auth Failure Detection', () => {
    it('should provide user-friendly message for missing auth', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const result = detectAuthFailure('authentication required');
+      const result = detectAuthFailure('[CLI] authentication required');

      expect(result.isAuthFailure).toBe(true);
      expect(result.message).toContain('Settings');
@@ -413,7 +413,7 @@ describe('Auth Failure Detection', () => {
    it('should provide user-friendly message for expired auth', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const result = detectAuthFailure('session expired');
+      const result = detectAuthFailure('Please obtain a new token - session expired');

      expect(result.isAuthFailure).toBe(true);
      expect(result.message).toContain('expired');
@@ -423,7 +423,7 @@ describe('Auth Failure Detection', () => {
    it('should provide user-friendly message for invalid auth', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const result = detectAuthFailure('unauthorized');
+      const result = detectAuthFailure('Error: unauthorized');

      expect(result.isAuthFailure).toBe(true);
      expect(result.message).toContain('Invalid');
@@ -434,10 +434,10 @@ describe('Auth Failure Detection', () => {
    it('should return true for auth failure errors', async () => {
      const { isAuthFailureError } = await import('../rate-limit-detector');

-      expect(isAuthFailureError('authentication required')).toBe(true);
-      expect(isAuthFailureError('not authenticated')).toBe(true);
-      expect(isAuthFailureError('unauthorized')).toBe(true);
-      expect(isAuthFailureError('invalid token')).toBe(true);
+      expect(isAuthFailureError('[CLI] authentication required')).toBe(true);
+      expect(isAuthFailureError('[Auth] not authenticated')).toBe(true);
+      expect(isAuthFailureError('Error: unauthorized')).toBe(true);
+      expect(isAuthFailureError('Error: invalid token')).toBe(true);
    });

    it('should return false for non-auth-failure errors', async () => {
@@ -454,12 +454,12 @@ describe('Auth Failure Detection', () => {
      const { detectRateLimit } = await import('../rate-limit-detector');

      const authErrors = [
-        'authentication required',
-        'not authenticated',
-        'unauthorized',
-        'invalid token',
-        'session expired',
-        'please log in'
+        '[CLI] authentication required',
+        '[Auth] not authenticated',
+        'Error: unauthorized',
+        'Error: invalid token',
+        'Please obtain a new token - session expired',
+        '· Please run /login'
      ];

      for (const error of authErrors) {
@@ -503,12 +503,12 @@ Please authenticate and try again.`;
      const { detectAuthFailure } = await import('../rate-limit-detector');

      const testCases = [
-        'AUTHENTICATION REQUIRED',
-        'Authentication Required',
-        'UNAUTHORIZED',
-        'Unauthorized',
-        'NOT AUTHENTICATED',
-        'Not Authenticated'
+        '[CLI] AUTHENTICATION REQUIRED',
+        '[Auth] Authentication Required',
+        'ERROR: UNAUTHORIZED',
+        'Error: Unauthorized',
+        '[API] NOT AUTHENTICATED',
+        '[Error] Not Authenticated'
      ];

      for (const output of testCases) {
@@ -538,7 +538,7 @@ Please authenticate and try again.`;
    it('should handle JSON error responses', async () => {
      const { detectAuthFailure } = await import('../rate-limit-detector');

-      const output = '{"error": "unauthorized", "message": "Please authenticate"}';
+      const output = '{"type":"authentication_error", "error": "unauthorized", "message": "Please authenticate"}';
      const result = detectAuthFailure(output);

      expect(result.isAuthFailure).toBe(true);
@@ -505,11 +505,11 @@ export interface PRReviewProgress {
 interface CIWaitResult {
  /** Whether we successfully waited (no timeout) */
  success: boolean;
-  /** Whether any checks are still in progress */
+  /** Whether any checks are still pending (queued or in_progress) */
  hasInProgress: boolean;
-  /** Number of checks currently in progress */
+  /** Number of checks currently pending (queued or in_progress) */
  inProgressCount: number;
-  /** Names of checks still in progress (if any) */
+  /** Names of checks still pending (if any) */
  inProgressChecks: string[];
  /** Whether we timed out waiting */
  timedOut: boolean;
@@ -520,9 +520,13 @@ interface CIWaitResult {
 /**
 * Wait for CI checks to complete before starting AI review.
 *
- * Polls GitHub API to check if any CI checks are "in_progress".
- * Only blocks on "in_progress" status - does NOT block on "queued" status
- * (which could be CLA, licensing workflows that may never run).
+ * Polls GitHub API to check if any CI checks are "queued" or "in_progress".
+ * Blocks on BOTH statuses because:
+ * - "queued" = CI has been triggered but not started yet
+ * - "in_progress" = CI is actively running
+ *
+ * We wait for all checks to reach "completed" status before reviewing,
+ * so our review doesn't report "CI is pending" when it will finish soon.
 *
 * @param token GitHub API token
 * @param repo Repository in "owner/repo" format
@@ -581,10 +585,10 @@ async function waitForCIChecks(
        }>;
      };

-      // Find checks that are actively running (in_progress)
-      // We do NOT block on "queued" status - those could be CLA/licensing workflows
+      // Find checks that are not yet completed (queued or in_progress)
+      // We block on BOTH statuses to ensure CI is fully done before reviewing
      const inProgressChecks = checkRuns.check_runs.filter(
-        (cr) => cr.status === "in_progress"
+        (cr) => cr.status === "queued" || cr.status === "in_progress"
      );

      const inProgressCount = inProgressChecks.length;
@@ -602,10 +606,10 @@ async function waitForCIChecks(
        inProgressNames,
      });

-      // If no checks are in_progress, we can proceed
+      // If no checks are pending (queued or in_progress), we can proceed
      if (inProgressCount === 0) {
        const waitTimeSeconds = Math.floor((Date.now() - startTime) / 1000);
-        debugLog("No CI checks in progress, proceeding with review", {
+        debugLog("All CI checks completed, proceeding with review", {
          prNumber,
          waitTimeSeconds,
        });
@@ -26,29 +26,31 @@ const RATE_LIMIT_INDICATORS = [
 /**
 * Patterns that indicate authentication failures
 * These patterns detect when Claude CLI/SDK fails due to missing or invalid auth
+ *
+ * IMPORTANT: These patterns must be specific enough to NOT match on AI response
+ * content that discusses authentication topics (e.g., PRs about auth features).
+ * The patterns should only match actual API error messages.
 */
 const AUTH_FAILURE_PATTERNS = [
-  /authentication\s*(is\s*)?required/i,
-  /not\s*(yet\s*)?authenticated/i,
-  /login\s*(is\s*)?required/i,
-  /oauth\s*token\s*(is\s*)?(invalid|expired|missing)/i,
-  /unauthorized/i,
-  /please\s*(log\s*in|login|authenticate)/i,
-  /invalid\s*(credentials|token|api\s*key)/i,
-  /auth(entication)?\s+(failed|error|failure)/i,
-  /session\s*(expired|invalid)/i,
-  /access\s*denied/i,
-  /permission\s*denied/i,
-  /401\s*unauthorized/i,
-  /credentials\s*(are\s*)?(missing|invalid|expired)/i,
-  // Match "OAuth token has expired" format from Claude API
-  /oauth\s*token\s+has\s+expired/i,
-  // Match Claude API authentication_error type in JSON responses
+  // Match Claude API authentication_error type in JSON responses (most reliable)
  /["']?type["']?\s*:\s*["']?authentication_error["']?/i,
-  // Match plain "API Error: 401" without requiring "unauthorized"
+  // Match plain "API Error: 401" - this is a structured error format
  /API\s*Error:\s*401/i,
-  // Match "Please obtain a new token" message from Claude API
-  /please\s*(obtain|get|refresh)\s*(a\s*)?new\s*token/i
+  // Match "OAuth token has expired" format from Claude API (specific phrasing)
+  /oauth\s*token\s+has\s+expired/i,
+  // Match "Please obtain a new token" or "refresh your existing token" - API specific
+  /please\s+(obtain\s+a\s+new|refresh\s+your)\s+(existing\s+)?token/i,
+  // Match Claude CLI specific auth messages (with context markers)
+  /\[.*\]\s*authentication\s*(is\s*)?required/i,
+  /\[.*\]\s*not\s*(yet\s*)?authenticated/i,
+  /\[.*\]\s*login\s*(is\s*)?required/i,
+  // Match 401 status codes in structured error output
+  /status[:\s]+401/i,
+  /HTTP\s*401/i,
+  // Match specific error prefixes that indicate actual errors (not AI discussion)
+  /Error:\s*.*(?:unauthorized|authentication|invalid\s*token)/i,
+  // Match · Please run /login format from Claude CLI
+  /·\s*Please\s+run\s+\/login/i,
 ];

 /**
@@ -1,11 +1,15 @@
 """
-Integration Tests for PR Review System - Phase 4
-=================================================
+Integration Tests for PR Review System - Phase 4+
+==================================================

-Tests validating all Phase 1-3 features work correctly:
- Phase 1: Confidence routing, evidence validation, scope filtering
+Tests validating key features:
 - Phase 2: Import detection (path aliases, Python), reverse dependencies
 - Phase 3: Multi-agent cross-validation
+- Phase 5+: Scope filtering with is_impact_finding schema field
+
+Note: ConfidenceTier and _validate_finding_evidence were removed in Phase 5
+(Code Simplification). Evidence validation is now handled by schema enforcement
+and the finding-validator agent.
 """

 import sys
@@ -71,8 +75,16 @@ sys.modules['services.pydantic_models'] = pydantic_models_module
 pydantic_models_spec.loader.exec_module(pydantic_models_module)
 AgentAgreement = pydantic_models_module.AgentAgreement

+# Load agent_utils (shared utility for working directory injection)
+agent_utils_spec = importlib.util.spec_from_file_location(
+    "agent_utils",
+    backend_path / "runners" / "github" / "services" / "agent_utils.py"
+)
+agent_utils_module = importlib.util.module_from_spec(agent_utils_spec)
+sys.modules['services.agent_utils'] = agent_utils_module
+agent_utils_spec.loader.exec_module(agent_utils_module)

-# Load parallel_orchestrator_reviewer (contains ConfidenceTier, validation functions)
+# Load parallel_orchestrator_reviewer (contains _is_finding_in_scope and _cross_validate_findings)
 orchestrator_spec = importlib.util.spec_from_file_location(
    "parallel_orchestrator_reviewer",
    backend_path / "runners" / "github" / "services" / "parallel_orchestrator_reviewer.py"
@@ -99,186 +111,33 @@ for name in _modules_to_mock:
        sys.modules[name] = _original_modules[name]
    elif name in sys.modules:
        del sys.modules[name]
-ConfidenceTier = orchestrator_module.ConfidenceTier
-_validate_finding_evidence = orchestrator_module._validate_finding_evidence
+
+# Import only functions that still exist after Phase 5
 _is_finding_in_scope = orchestrator_module._is_finding_in_scope


 # =============================================================================
-# Phase 1 Tests: Confidence Routing, Evidence Validation, Scope Filtering
+# Phase 5+ Tests: Scope Filtering (Updated)
 # =============================================================================

-class TestConfidenceTierRouting:
-    """Test confidence tier routing logic (Phase 1)."""
-
-    def test_high_confidence_tier(self):
-        """Verify confidence >= 0.8 returns HIGH tier."""
-        assert ConfidenceTier.get_tier(0.8) == ConfidenceTier.HIGH
-        assert ConfidenceTier.get_tier(0.85) == ConfidenceTier.HIGH
-        assert ConfidenceTier.get_tier(0.95) == ConfidenceTier.HIGH
-        assert ConfidenceTier.get_tier(1.0) == ConfidenceTier.HIGH
-
-    def test_medium_confidence_tier(self):
-        """Verify confidence 0.5-0.8 returns MEDIUM tier."""
-        assert ConfidenceTier.get_tier(0.5) == ConfidenceTier.MEDIUM
-        assert ConfidenceTier.get_tier(0.6) == ConfidenceTier.MEDIUM
-        assert ConfidenceTier.get_tier(0.7) == ConfidenceTier.MEDIUM
-        assert ConfidenceTier.get_tier(0.79) == ConfidenceTier.MEDIUM
-
-    def test_low_confidence_tier(self):
-        """Verify confidence < 0.5 returns LOW tier."""
-        assert ConfidenceTier.get_tier(0.0) == ConfidenceTier.LOW
-        assert ConfidenceTier.get_tier(0.1) == ConfidenceTier.LOW
-        assert ConfidenceTier.get_tier(0.3) == ConfidenceTier.LOW
-        assert ConfidenceTier.get_tier(0.49) == ConfidenceTier.LOW
-
-    def test_boundary_values(self):
-        """Test exact boundary values: 0.5 (MEDIUM) and 0.8 (HIGH)."""
-        # 0.5 is MEDIUM threshold (inclusive)
-        assert ConfidenceTier.get_tier(0.5) == ConfidenceTier.MEDIUM
-        # 0.8 is HIGH threshold (inclusive)
-        assert ConfidenceTier.get_tier(0.8) == ConfidenceTier.HIGH
-        # Just below boundaries
-        assert ConfidenceTier.get_tier(0.4999) == ConfidenceTier.LOW
-        assert ConfidenceTier.get_tier(0.7999) == ConfidenceTier.MEDIUM
-
-    def test_tier_constants_values(self):
-        """Verify tier constant values match expected strings."""
-        assert ConfidenceTier.HIGH == "high"
-        assert ConfidenceTier.MEDIUM == "medium"
-        assert ConfidenceTier.LOW == "low"
-
-    def test_threshold_values(self):
-        """Verify threshold values through behavior (0.8 for HIGH, 0.5 for LOW)."""
-        # HIGH threshold is 0.8
-        assert ConfidenceTier.get_tier(0.8) == ConfidenceTier.HIGH
-        assert ConfidenceTier.get_tier(0.79) == ConfidenceTier.MEDIUM
-
-        # LOW threshold is 0.5
-        assert ConfidenceTier.get_tier(0.5) == ConfidenceTier.MEDIUM
-        assert ConfidenceTier.get_tier(0.49) == ConfidenceTier.LOW
-
-
-class TestEvidenceValidation:
-    """Test evidence validation logic (Phase 1)."""
-
-    @pytest.fixture
-    def make_finding(self):
-        """Factory fixture to create PRReviewFinding instances."""
-        def _make_finding(evidence: str | None = None, **kwargs):
-            defaults = {
-                "id": "TEST001",
-                "severity": ReviewSeverity.MEDIUM,
-                "category": ReviewCategory.QUALITY,
-                "title": "Test Finding",
-                "description": "Test description",
-                "file": "src/test.py",
-                "line": 10,
-                "evidence": evidence,
-            }
-            defaults.update(kwargs)
-            return PRReviewFinding(**defaults)
-        return _make_finding
-
-    def test_valid_evidence_with_code_syntax(self, make_finding):
-        """Evidence with =, (), {} should pass validation."""
-        # Assignment operator
-        finding = make_finding(evidence="const x = getValue()")
-        is_valid, reason = _validate_finding_evidence(finding)
-        assert is_valid, f"Failed: {reason}"
-
-        # Function call
-        finding = make_finding(evidence="someFunction(arg1, arg2)")
-        is_valid, reason = _validate_finding_evidence(finding)
-        assert is_valid, f"Failed: {reason}"
-
-        # Object/dict literal
-        finding = make_finding(evidence="config = { 'key': 'value' }")
-        is_valid, reason = _validate_finding_evidence(finding)
-        assert is_valid, f"Failed: {reason}"
-
-    def test_invalid_evidence_no_code_syntax(self, make_finding):
-        """Prose-only evidence without code syntax should fail."""
-        finding = make_finding(evidence="This code is problematic and needs fixing")
-        is_valid, reason = _validate_finding_evidence(finding)
-        assert not is_valid
-        assert "lacks code syntax" in reason.lower()
-
-    def test_empty_evidence_fails(self, make_finding):
-        """Empty or short evidence should fail validation."""
-        # No evidence
-        finding = make_finding(evidence=None)
-        is_valid, reason = _validate_finding_evidence(finding)
-        assert not is_valid
-        assert "no evidence" in reason.lower()
-
-        # Empty string
-        finding = make_finding(evidence="")
-        is_valid, reason = _validate_finding_evidence(finding)
-        assert not is_valid
-
-        # Too short (< 10 chars)
-        finding = make_finding(evidence="x = 1")
-        is_valid, reason = _validate_finding_evidence(finding)
-        assert not is_valid
-        assert "too short" in reason.lower()
-
-    def test_evidence_with_function_def(self, make_finding):
-        """Evidence with 'def ' or 'function ' patterns should pass."""
-        # Python function definition
-        finding = make_finding(evidence="def vulnerable_function(user_input):")
-        is_valid, reason = _validate_finding_evidence(finding)
-        assert is_valid, f"Failed: {reason}"
-
-        # JavaScript function
-        finding = make_finding(evidence="function handleRequest(req, res) {")
-        is_valid, reason = _validate_finding_evidence(finding)
-        assert is_valid, f"Failed: {reason}"
-
-    def test_evidence_rejects_description_patterns(self, make_finding):
-        """Evidence starting with vague patterns should be rejected."""
-        patterns = [
-            "The code has an issue with security",
-            "This function could be improved",
-            "It appears there is a vulnerability",
-            "Seems to be missing error handling",
-        ]
-        for pattern in patterns:
-            finding = make_finding(evidence=pattern)
-            is_valid, reason = _validate_finding_evidence(finding)
-            assert not is_valid, f"Should reject: {pattern}"
-            assert "description pattern" in reason.lower() or "lacks code" in reason.lower()
-
-    def test_evidence_with_various_syntax_chars(self, make_finding):
-        """Test various code syntax characters are recognized."""
-        # Semicolon
-        finding = make_finding(evidence="let x = 5; let y = 10;")
-        is_valid, _ = _validate_finding_evidence(finding)
-        assert is_valid
-
-        # Colon (Python dict/type hint)
-        finding = make_finding(evidence="config: Dict[str, int]")
-        is_valid, _ = _validate_finding_evidence(finding)
-        assert is_valid
-
-        # Arrow
-        finding = make_finding(evidence="result->getValue()")
-        is_valid, _ = _validate_finding_evidence(finding)
-        assert is_valid
-
-        # Brackets
-        finding = make_finding(evidence="array[0] = items[index]")
-        is_valid, _ = _validate_finding_evidence(finding)
-        assert is_valid
-
-
 class TestScopeFiltering:
-    """Test scope filtering logic (Phase 1)."""
+    """Test scope filtering logic (updated for Phase 5 - uses is_impact_finding schema field)."""

    @pytest.fixture
    def make_finding(self):
-        """Factory fixture to create PRReviewFinding instances."""
-        def _make_finding(file: str = "src/test.py", line: int = 10, **kwargs):
+        """Factory fixture to create PRReviewFinding instances.
+
+        Note: is_impact_finding is set as an attribute after creation because
+        PRReviewFinding (dataclass) doesn't have this field - it's on the
+        ParallelOrchestratorFinding Pydantic model. The actual code uses
+        getattr(finding, 'is_impact_finding', False) to access it.
+        """
+        def _make_finding(
+            file: str = "src/test.py",
+            line: int = 10,
+            is_impact_finding: bool = False,
+            **kwargs
+        ):
            defaults = {
                "id": "TEST001",
                "severity": ReviewSeverity.MEDIUM,
@@ -289,7 +148,10 @@ class TestScopeFiltering:
                "line": line,
            }
            defaults.update(kwargs)
-            return PRReviewFinding(**defaults)
+            finding = PRReviewFinding(**defaults)
+            # Set is_impact_finding as attribute (accessed via getattr in _is_finding_in_scope)
+            finding.is_impact_finding = is_impact_finding
+            return finding
        return _make_finding

    def test_finding_in_changed_files_passes(self, make_finding):
@@ -329,35 +191,33 @@ class TestScopeFiltering:
        assert not is_valid

    def test_impact_finding_allowed_for_unchanged_files(self, make_finding):
-        """Finding with impact keywords should be allowed for unchanged files."""
+        """Finding with is_impact_finding=True should be allowed for unchanged files."""
        changed_files = ["src/auth.py"]

-        # 'breaks' keyword
+        # Impact finding for unchanged file
        finding = make_finding(
            file="src/utils.py",
            line=10,
+            is_impact_finding=True,  # Schema field replaces keyword detection
            description="This change breaks the helper function in utils.py"
        )
        is_valid, _ = _is_finding_in_scope(finding, changed_files)
        assert is_valid

-        # 'affects' keyword
-        finding = make_finding(
-            file="src/config.py",
-            line=5,
-            description="Changes in auth.py affects config loading"
-        )
-        is_valid, _ = _is_finding_in_scope(finding, changed_files)
-        assert is_valid
+    def test_non_impact_finding_filtered_for_unchanged_files(self, make_finding):
+        """Finding with is_impact_finding=False should be filtered for unchanged files."""
+        changed_files = ["src/auth.py"]

-        # 'depends' keyword
+        # Non-impact finding for unchanged file
        finding = make_finding(
            file="src/database.py",
            line=20,
+            is_impact_finding=False,
            description="database.py depends on modified auth module"
        )
-        is_valid, _ = _is_finding_in_scope(finding, changed_files)
-        assert is_valid
+        is_valid, reason = _is_finding_in_scope(finding, changed_files)
+        assert not is_valid
+        assert "not in pr changed files" in reason.lower()

    def test_no_file_specified_fails(self, make_finding):
        """Finding with no file specified should fail."""
@@ -564,8 +424,8 @@ class TestReverseDepDetection:
        # Should NOT include standalone.ts
        assert not any("standalone.ts" in d for d in dependents)

-    def test_skips_generic_names(self, tmp_path):
-        """Generic names (index, main, utils) should be skipped to reduce noise."""
+    def test_generic_names_not_skipped(self, tmp_path):
+        """Generic names (index, main, utils) are no longer skipped - LLM decides relevance."""
        src_dir = tmp_path / "src"
        src_dir.mkdir()

@@ -575,13 +435,12 @@ class TestReverseDepDetection:

        gatherer = PRContextGathererIsolated(tmp_path, pr_number=1)

-        # Generic names should return empty set (skipped)
+        # Generic names should NOT be skipped anymore (behavior changed in Phase 4)
+        # The LLM-driven system decides what's relevant based on PR context
        dependents_index = gatherer._find_dependents("src/index.ts")
-        dependents_main = gatherer._find_dependents("src/main.ts")

-        # These should be skipped due to generic names
-        assert len(dependents_index) == 0
-        assert len(dependents_main) == 0
+        # main.ts imports index, so it should be found as a dependent
+        assert "src/main.ts" in dependents_index

    def test_respects_file_limit(self, tmp_path):
        """Large repo search should stop after reaching file limit."""
@@ -831,302 +690,10 @@ class TestCrossValidation:
        # Should keep CRITICAL (highest severity)
        assert validated[0].severity == ReviewSeverity.CRITICAL

-
-# =============================================================================
-# Integration Verification Tests: Full Pipeline Tests
-# =============================================================================
-
-class TestIntegrationPipeline:
-    """Test complete pipeline integration of Phase 1-3 features."""
-
-    @pytest.fixture
-    def make_finding(self):
-        """Factory fixture to create PRReviewFinding instances."""
-        def _make_finding(
-            id: str = "TEST001",
-            file: str = "src/test.py",
-            line: int = 10,
-            category: ReviewCategory = ReviewCategory.SECURITY,
-            severity: ReviewSeverity = ReviewSeverity.HIGH,
-            confidence: float = 0.7,
-            evidence: str = "const x = getValue()",
-            source_agents: list = None,
-            **kwargs
-        ):
-            return PRReviewFinding(
-                id=id,
-                severity=severity,
-                category=category,
-                title=kwargs.get("title", "Test Finding"),
-                description=kwargs.get("description", "Test description"),
-                file=file,
-                line=line,
-                confidence=confidence,
-                evidence=evidence,
-                source_agents=source_agents or [],
-                **{k: v for k, v in kwargs.items() if k not in ["title", "description"]}
-            )
-        return _make_finding
-
-    @pytest.fixture
-    def mock_reviewer(self, tmp_path):
-        """Create a mock ParallelOrchestratorReviewer instance."""
-        from models import GitHubRunnerConfig
-
-        config = GitHubRunnerConfig(
-            token="test-token",
-            repo="test/repo"
-        )
-        github_dir = tmp_path / ".auto-claude" / "github"
-        github_dir.mkdir(parents=True)
-
-        reviewer = ParallelOrchestratorReviewer(
-            project_dir=tmp_path,
-            github_dir=github_dir,
-            config=config
-        )
-        return reviewer
-
-    def test_pipeline_flow_high_confidence_valid_evidence_in_scope(
-        self, make_finding, mock_reviewer
-    ):
-        """Test complete flow: high confidence + valid evidence + in scope passes all checks."""
-        changed_files = ["src/auth.py"]
-        finding = make_finding(
-            file="src/auth.py",
-            line=15,
-            confidence=0.85,  # HIGH tier (>= 0.8)
-            evidence="const password = req.body.password; query(`SELECT * WHERE pwd='${password}'`)",
-        )
-
-        # Phase 1a: Confidence tier
-        tier = ConfidenceTier.get_tier(finding.confidence)
-        assert tier == ConfidenceTier.HIGH
-
-        # Phase 1b: Evidence validation
-        is_valid_evidence, _ = _validate_finding_evidence(finding)
-        assert is_valid_evidence
-
-        # Phase 1c: Scope filtering
-        is_in_scope, _ = _is_finding_in_scope(finding, changed_files)
-        assert is_in_scope
-
-        # All checks pass - finding should be included in review
-
-    def test_pipeline_flow_low_confidence_filtered(self, make_finding):
-        """Test that low confidence findings are filtered even with valid evidence."""
-        changed_files = ["src/auth.py"]
-        finding = make_finding(
-            file="src/auth.py",
-            line=15,
-            confidence=0.3,  # LOW tier (< 0.5)
-            evidence="const x = getValue()",
-        )
-
-        tier = ConfidenceTier.get_tier(finding.confidence)
-        assert tier == ConfidenceTier.LOW
-
-        # Low confidence findings may be filtered based on tier routing
-        # This test documents the expected behavior
-
-    def test_pipeline_flow_cross_validation_elevates_medium_to_high(
-        self, make_finding, mock_reviewer
-    ):
-        """Test that cross-validation can elevate MEDIUM tier to HIGH tier."""
-        # Two agents find same issue with MEDIUM confidence
-        # Give finding2 CRITICAL severity so it becomes the primary (sorted first)
-        finding1 = make_finding(
-            id="F1",
-            file="src/auth.py",
-            line=10,
-            confidence=0.6,  # MEDIUM tier
-            severity=ReviewSeverity.HIGH,
-            source_agents=["security-reviewer"],
-        )
-        finding2 = make_finding(
-            id="F2",
-            file="src/auth.py",
-            line=10,
-            confidence=0.7,  # MEDIUM tier - this will be primary (higher severity)
-            severity=ReviewSeverity.CRITICAL,
-            source_agents=["quality-reviewer"],
-        )
-
-        # Before cross-validation: both MEDIUM
-        assert ConfidenceTier.get_tier(finding1.confidence) == ConfidenceTier.MEDIUM
-        assert ConfidenceTier.get_tier(finding2.confidence) == ConfidenceTier.MEDIUM
-
-        # Cross-validate
-        validated, _ = mock_reviewer._cross_validate_findings([finding1, finding2])
-
-        # After cross-validation: boosted to primary.confidence + 0.15 = 0.7 + 0.15 = 0.85 (HIGH tier)
-        # Note: The code uses primary finding's confidence, sorted by severity (CRITICAL first)
-        assert validated[0].confidence == pytest.approx(0.85, rel=0.01)
-        assert ConfidenceTier.get_tier(validated[0].confidence) == ConfidenceTier.HIGH
-        assert validated[0].severity == ReviewSeverity.CRITICAL  # Highest severity preserved
-
-    def test_pipeline_invalid_evidence_rejected(self, make_finding):
-        """Test that findings with invalid evidence are rejected regardless of confidence."""
-        finding = make_finding(
-            file="src/auth.py",
-            line=15,
-            confidence=0.95,  # HIGH confidence
-            evidence="This code looks problematic",  # Prose, not code
-        )
-
-        is_valid_evidence, reason = _validate_finding_evidence(finding)
-        assert not is_valid_evidence
-        assert "lacks code syntax" in reason.lower()
-
-    def test_pipeline_out_of_scope_rejected(self, make_finding):
-        """Test that findings outside changed files are rejected."""
-        changed_files = ["src/auth.py", "src/utils.py"]
-        finding = make_finding(
-            file="src/database.py",  # Not in changed files
-            line=15,
-            confidence=0.95,
-            evidence="const query = buildQuery(input)",
-        )
-
-        is_in_scope, reason = _is_finding_in_scope(finding, changed_files)
-        assert not is_in_scope
-
-    def test_pipeline_impact_finding_allowed(self, make_finding):
-        """Test that impact findings for unchanged files are allowed."""
-        changed_files = ["src/auth.py"]
-        finding = make_finding(
-            file="src/database.py",  # Not in changed files
-            line=15,
-            confidence=0.85,
-            evidence="const query = buildQuery(input)",
-            description="Changes to auth.py breaks the database connection in database.py",
-        )
-
-        is_in_scope, _ = _is_finding_in_scope(finding, changed_files)
-        assert is_in_scope  # Allowed because description mentions impact
-
-    def test_pipeline_end_to_end_review_scenario(self, make_finding, mock_reviewer):
-        """Test realistic end-to-end review scenario with multiple findings."""
-        changed_files = ["src/auth.py", "src/api.py"]
-
-        # Findings from multiple agents
-        findings = [
-            # High-confidence security finding from agent 1
-            make_finding(
-                id="SEC001",
-                file="src/auth.py",
-                line=42,
-                category=ReviewCategory.SECURITY,
-                severity=ReviewSeverity.CRITICAL,
-                confidence=0.85,
-                evidence="password = req.body.password; db.query(`SELECT * WHERE pwd='${password}'`)",
-                source_agents=["security-reviewer"],
-                description="SQL injection in login",
-            ),
-            # Same finding from agent 2 (will be cross-validated)
-            make_finding(
-                id="SEC002",
-                file="src/auth.py",
-                line=42,
-                category=ReviewCategory.SECURITY,
-                severity=ReviewSeverity.HIGH,
-                confidence=0.75,
-                evidence="db.query(`SELECT * WHERE pwd='${password}'`)",
-                source_agents=["quality-reviewer"],
-                description="Unsanitized query parameter",
-            ),
-            # Valid quality finding
-            make_finding(
-                id="QUAL001",
-                file="src/api.py",
-                line=100,
-                category=ReviewCategory.QUALITY,
-                severity=ReviewSeverity.MEDIUM,
-                confidence=0.7,
-                evidence="function handleRequest(req) { /* missing error handling */ }",
-                source_agents=["quality-reviewer"],
-                description="Missing error handling",
-            ),
-            # Invalid: out of scope
-            make_finding(
-                id="OUT001",
-                file="src/database.py",  # Not in changed files
-                line=10,
-                category=ReviewCategory.SECURITY,
-                confidence=0.9,
-                evidence="connection.close()",
-                source_agents=["security-reviewer"],
-            ),
-            # Invalid: prose evidence
-            make_finding(
-                id="PROSE001",
-                file="src/auth.py",
-                line=50,
-                category=ReviewCategory.QUALITY,
-                confidence=0.8,
-                evidence="This code should be refactored for better maintainability",
-                source_agents=["quality-reviewer"],
-            ),
-        ]
-
-        # Step 1: Cross-validate
-        validated, agreement = mock_reviewer._cross_validate_findings(findings)
-
-        # SEC001 and SEC002 should be merged (same file, line, category)
-        security_findings = [f for f in validated if f.category == ReviewCategory.SECURITY
-                           and f.file == "src/auth.py" and f.line == 42]
-        assert len(security_findings) == 1
-        # Cross-validated finding should have boosted confidence
-        assert security_findings[0].cross_validated is True
-        assert security_findings[0].confidence >= 0.85  # Boosted from max(0.85, 0.75) + 0.15
-
-        # Step 2: Validate evidence for each finding
-        valid_evidence_findings = []
-        for f in validated:
-            is_valid, _ = _validate_finding_evidence(f)
-            if is_valid:
-                valid_evidence_findings.append(f)
-
-        # PROSE001 should be filtered out (if present in validated)
-        prose_findings = [f for f in valid_evidence_findings
-                         if f.evidence and "refactored" in f.evidence]
-        assert len(prose_findings) == 0
-
-        # Step 3: Filter by scope
-        in_scope_findings = []
-        for f in valid_evidence_findings:
-            is_in_scope, _ = _is_finding_in_scope(f, changed_files)
-            if is_in_scope:
-                in_scope_findings.append(f)
-
-        # OUT001 should be filtered out (src/database.py not in changed_files)
-        database_findings = [f for f in in_scope_findings if f.file == "src/database.py"]
-        assert len(database_findings) == 0
-
-    def test_pipeline_empty_findings_handled(self, mock_reviewer):
+    def test_empty_findings_handled(self, mock_reviewer):
        """Test that empty findings list is handled gracefully."""
        validated, agreement = mock_reviewer._cross_validate_findings([])

        assert len(validated) == 0
        assert len(agreement.agreed_findings) == 0
-        assert len(agreement.conflicting_findings) == 0  # Note: uses conflicting_findings, not disputed
-
-    def test_pipeline_confidence_tier_determines_routing(self, make_finding):
-        """Test that confidence tier determines review routing behavior."""
-        # Document the tier routing expectations
-        test_cases = [
-            (0.95, ConfidenceTier.HIGH, "auto-include"),
-            (0.85, ConfidenceTier.HIGH, "auto-include"),
-            (0.80, ConfidenceTier.HIGH, "auto-include"),
-            (0.75, ConfidenceTier.MEDIUM, "needs verification"),
-            (0.60, ConfidenceTier.MEDIUM, "needs verification"),
-            (0.50, ConfidenceTier.MEDIUM, "needs verification"),
-            (0.45, ConfidenceTier.LOW, "consider filtering"),
-            (0.30, ConfidenceTier.LOW, "consider filtering"),
-            (0.10, ConfidenceTier.LOW, "consider filtering"),
-        ]
-
-        for confidence, expected_tier, expected_routing in test_cases:
-            finding = make_finding(confidence=confidence)
-            tier = ConfidenceTier.get_tier(finding.confidence)
-            assert tier == expected_tier, f"Confidence {confidence} should be {expected_tier}"
+        assert len(agreement.conflicting_findings) == 0
@@ -225,69 +225,6 @@ class TestLineNumberVerification:
        assert validator._is_line_relevant(line_content, finding)


-class TestFalsePositiveDetection:
-    """Test false positive detection."""
-
-    def test_vague_low_severity_filtered(self, validator):
-        """Test that vague low-severity findings are filtered."""
-        finding = PRReviewFinding(
-            id="STYLE001",
-            severity=ReviewSeverity.LOW,
-            category=ReviewCategory.STYLE,
-            title="Code Could Be Improved",
-            description="This code could be improved by considering using better practices.",
-            file="src/utils.py",
-            line=1,
-        )
-
-        assert validator._is_false_positive(finding)
-
-    def test_generic_without_fix_filtered(self, validator):
-        """Test that generic suggestions without fixes are filtered."""
-        finding = PRReviewFinding(
-            id="QUAL001",
-            severity=ReviewSeverity.LOW,
-            category=ReviewCategory.QUALITY,
-            title="Improve This Code",
-            description="This code should be improved for better quality and maintainability.",
-            file="src/utils.py",
-            line=1,
-            suggested_fix="Fix it",  # Too short
-        )
-
-        assert validator._is_false_positive(finding)
-
-    def test_style_without_suggestion_filtered(self, validator):
-        """Test that style findings without good suggestions are filtered."""
-        finding = PRReviewFinding(
-            id="STYLE002",
-            severity=ReviewSeverity.LOW,
-            category=ReviewCategory.STYLE,
-            title="Formatting Issue",
-            description="The formatting of this code doesn't follow best practices and should be adjusted.",
-            file="src/utils.py",
-            line=1,
-            suggested_fix="",  # No suggestion
-        )
-
-        assert validator._is_false_positive(finding)
-
-    def test_specific_high_severity_not_filtered(self, validator):
-        """Test that specific high-severity findings are not filtered."""
-        finding = PRReviewFinding(
-            id="SEC001",
-            severity=ReviewSeverity.HIGH,
-            category=ReviewCategory.SECURITY,
-            title="SQL Injection Vulnerability",
-            description="The query construction uses f-strings which allows SQL injection. An attacker could inject malicious SQL code through the username parameter.",
-            file="src/auth.py",
-            line=13,
-            suggested_fix="Use parameterized queries with placeholders instead of string formatting",
-        )
-
-        assert not validator._is_false_positive(finding)
-
-
 class TestActionabilityScoring:
    """Test actionability scoring."""

@@ -378,21 +315,17 @@ class TestConfidenceThreshold:
            severity=ReviewSeverity.LOW,
            category=ReviewCategory.STYLE,
            title="Styl",  # Very minimal (9 chars, just at min)
-            description="Could be improved with better formatting here",  # Vague pattern
+            description="Could be improved with better formatting here",
            file="src/utils.py",
            line=1,
            suggested_fix="",  # No fix
        )

-        # Should fail - low severity + vague + no fix + short title
-        # Score should be 0.5 (base) + 0.1 (file+line) + 0.1 (desc>50) = 0.7
-        # But vague pattern makes it a false positive, so it should fail validation before threshold check
-        # This test should check that the actionability score alone is insufficient
-        score = validator._score_actionability(finding)
+        # Score check: low severity with no fix gets low actionability
        # With no fix, short title, and low severity: 0.5 (base) + 0.1 (file+line) = 0.6
-        # But this still meets 0.6 threshold for low severity
-        # Let's check the finding gets filtered as false positive instead
-        assert validator._is_false_positive(finding)  # Should be filtered as FP
+        # This barely meets the 0.6 threshold for low severity
+        score = validator._score_actionability(finding)
+        assert score <= 0.6  # Low actionability due to missing suggested fix


 class TestFindingEnhancement:
@@ -39,6 +39,10 @@ from pydantic_models import (
    DeepAnalysisFinding,
    StructuralIssue,
    AICommentTriage,
+    # Verification evidence models (Phase 2)
+    VerificationEvidence,
+    ParallelOrchestratorFinding,
+    BaseFinding,
 )


@@ -93,6 +97,11 @@ class TestFollowupFinding:
            "line": 42,
            "suggested_fix": "Use parameterized queries",
            "fixable": True,
+            "verification": {
+                "code_examined": "query = 'SELECT * FROM users WHERE id=' + user_input",
+                "line_range_examined": [42, 42],
+                "verification_method": "direct_code_inspection",
+            },
        }
        result = FollowupFinding.model_validate(data)
        assert result.id == "new-1"
@@ -110,6 +119,11 @@ class TestFollowupFinding:
            "title": "Missing docstring",
            "description": "Function lacks documentation",
            "file": "utils.py",
+            "verification": {
+                "code_examined": "def process_data(data):\n    return data",
+                "line_range_examined": [1, 2],
+                "verification_method": "direct_code_inspection",
+            },
        }
        result = FollowupFinding.model_validate(data)
        assert result.line == 0  # Default
@@ -163,6 +177,11 @@ class TestFollowupReviewResponse:
                    "description": "Complex method",
                    "file": "service.py",
                    "line": 100,
+                    "verification": {
+                        "code_examined": "def process(self, data):\n    # 50 lines of nested if statements",
+                        "line_range_examined": [100, 150],
+                        "verification_method": "direct_code_inspection",
+                    },
                }
            ],
            "comment_findings": [],
@@ -233,6 +252,11 @@ class TestOrchestratorFinding:
            "severity": "medium",
            "suggestion": "Add error handling with proper logging",
            "evidence": "def handle_request(req):\n    result = db.query(req.id)  # no try-catch",
+            "verification": {
+                "code_examined": "def handle_request(req):\n    result = db.query(req.id)  # no try-catch",
+                "line_range_examined": [25, 26],
+                "verification_method": "direct_code_inspection",
+            },
        }
        result = OrchestratorFinding.model_validate(data)
        assert result.file == "src/api.py"
@@ -247,6 +271,11 @@ class TestOrchestratorFinding:
            "description": "Test finding",
            "category": "quality",
            "severity": "low",
+            "verification": {
+                "code_examined": "def test():\n    pass",
+                "line_range_examined": [1, 2],
+                "verification_method": "direct_code_inspection",
+            },
        }
        result = OrchestratorFinding.model_validate(data)
        assert result.evidence is None
@@ -269,6 +298,11 @@ class TestOrchestratorReviewResponse:
                    "category": "security",
                    "severity": "critical",
                    "evidence": "API_KEY = 'sk-prod-12345abcdef'",
+                    "verification": {
+                        "code_examined": "API_KEY = 'sk-prod-12345abcdef'",
+                        "line_range_examined": [10, 10],
+                        "verification_method": "direct_code_inspection",
+                    },
                }
            ],
            "summary": "Found 1 critical security issue",
@@ -370,6 +404,11 @@ class TestSecurityFinding:
            "description": "Unescaped user input",
            "file": "template.html",
            "line": 50,
+            "verification": {
+                "code_examined": "<div>{{ user_input }}</div>",
+                "line_range_examined": [50, 50],
+                "verification_method": "direct_code_inspection",
+            },
        }
        result = SecurityFinding.model_validate(data)
        assert result.category == "security"
@@ -389,6 +428,11 @@ class TestDeepAnalysisFinding:
            "line": 100,
            "category": "logic",
            "evidence": "shared_state += 1  # no lock protection",
+            "verification": {
+                "code_examined": "shared_state += 1  # no lock protection",
+                "line_range_examined": [100, 100],
+                "verification_method": "direct_code_inspection",
+            },
        }
        result = DeepAnalysisFinding.model_validate(data)
        assert result.evidence == "shared_state += 1  # no lock protection"
@@ -403,6 +447,11 @@ class TestDeepAnalysisFinding:
            "file": "lib.py",
            "category": "verification_failed",
            "verification_note": "Unable to find test coverage",
+            "verification": {
+                "code_examined": "def some_function():\n    return process_data()",
+                "line_range_examined": [1, 2],
+                "verification_method": "cross_file_trace",
+            },
        }
        result = DeepAnalysisFinding.model_validate(data)
        assert result.verification_note == "Unable to find test coverage"
@@ -441,3 +490,220 @@ class TestAICommentTriage:
            }
            result = AICommentTriage.model_validate(data)
            assert result.verdict == verdict
+
+
+# =============================================================================
+# Phase 2: Schema Enforcement Tests
+# =============================================================================
+
+
+class TestVerificationEvidence:
+    """Tests for VerificationEvidence model."""
+
+    def test_valid_verification(self):
+        """Test valid verification evidence."""
+        data = {
+            "code_examined": "def process_input(user_input):\n    return eval(user_input)",
+            "line_range_examined": [10, 11],
+            "verification_method": "direct_code_inspection",
+        }
+        result = VerificationEvidence.model_validate(data)
+        assert "eval" in result.code_examined
+        assert result.line_range_examined == [10, 11]
+        assert result.verification_method == "direct_code_inspection"
+
+    def test_empty_code_examined_rejected(self):
+        """Test that empty code_examined is rejected."""
+        data = {
+            "code_examined": "",
+            "line_range_examined": [1, 5],
+            "verification_method": "direct_code_inspection",
+        }
+        with pytest.raises(ValidationError) as exc_info:
+            VerificationEvidence.model_validate(data)
+        assert "code_examined" in str(exc_info.value)
+
+    def test_invalid_line_range_rejected(self):
+        """Test that invalid line ranges are rejected."""
+        data = {
+            "code_examined": "some code",
+            "line_range_examined": [1],  # Should have exactly 2 elements
+            "verification_method": "direct_code_inspection",
+        }
+        with pytest.raises(ValidationError) as exc_info:
+            VerificationEvidence.model_validate(data)
+        assert "line_range_examined" in str(exc_info.value)
+
+    def test_invalid_verification_method_rejected(self):
+        """Test that invalid verification method is rejected."""
+        data = {
+            "code_examined": "some code",
+            "line_range_examined": [1, 5],
+            "verification_method": "guessed",  # Invalid method
+        }
+        with pytest.raises(ValidationError) as exc_info:
+            VerificationEvidence.model_validate(data)
+        assert "verification_method" in str(exc_info.value)
+
+    def test_all_verification_methods(self):
+        """Test all valid verification methods."""
+        methods = [
+            "direct_code_inspection",
+            "cross_file_trace",
+            "test_verification",
+            "dependency_analysis",
+        ]
+        for method in methods:
+            data = {
+                "code_examined": "code",
+                "line_range_examined": [1, 5],
+                "verification_method": method,
+            }
+            result = VerificationEvidence.model_validate(data)
+            assert result.verification_method == method
+
+
+class TestParallelOrchestratorFindingVerification:
+    """Tests for verification field requirement on ParallelOrchestratorFinding."""
+
+    def test_missing_verification_rejected(self):
+        """Test that findings without verification are rejected."""
+        data = {
+            "id": "test-1",
+            "file": "test.py",
+            "line": 10,
+            "title": "Test finding",
+            "description": "A test finding without verification",
+            "category": "quality",
+            "severity": "medium",
+            # No verification field - should fail
+        }
+        with pytest.raises(ValidationError) as exc_info:
+            ParallelOrchestratorFinding.model_validate(data)
+        assert "verification" in str(exc_info.value)
+
+    def test_valid_finding_with_verification(self):
+        """Test valid finding with verification evidence."""
+        data = {
+            "id": "test-1",
+            "file": "test.py",
+            "line": 10,
+            "title": "SQL Injection vulnerability",
+            "description": "User input passed directly to query",
+            "category": "security",
+            "severity": "critical",
+            "verification": {
+                "code_examined": "cursor.execute(f'SELECT * FROM users WHERE id={user_id}')",
+                "line_range_examined": [10, 10],
+                "verification_method": "direct_code_inspection",
+            },
+        }
+        result = ParallelOrchestratorFinding.model_validate(data)
+        assert result.verification.code_examined is not None
+        assert result.verification.verification_method == "direct_code_inspection"
+
+    def test_is_impact_finding_default_false(self):
+        """Test is_impact_finding defaults to False."""
+        data = {
+            "id": "test-1",
+            "file": "test.py",
+            "line": 10,
+            "title": "Test",
+            "description": "Test",
+            "category": "quality",
+            "severity": "medium",
+            "verification": {
+                "code_examined": "code",
+                "line_range_examined": [10, 10],
+                "verification_method": "direct_code_inspection",
+            },
+        }
+        result = ParallelOrchestratorFinding.model_validate(data)
+        assert result.is_impact_finding is False
+
+    def test_is_impact_finding_true(self):
+        """Test is_impact_finding can be set True."""
+        data = {
+            "id": "test-1",
+            "file": "caller.py",
+            "line": 50,
+            "title": "Breaking change affects caller",
+            "description": "This file calls the changed function and will break",
+            "category": "logic",
+            "severity": "high",
+            "is_impact_finding": True,
+            "verification": {
+                "code_examined": "result = changed_function(x)",
+                "line_range_examined": [50, 50],
+                "verification_method": "cross_file_trace",
+            },
+        }
+        result = ParallelOrchestratorFinding.model_validate(data)
+        assert result.is_impact_finding is True
+
+    def test_checked_for_handling_elsewhere_default_false(self):
+        """Test checked_for_handling_elsewhere defaults to False."""
+        data = {
+            "id": "test-1",
+            "file": "test.py",
+            "line": 10,
+            "title": "Missing error handling",
+            "description": "No try-catch",
+            "category": "quality",
+            "severity": "medium",
+            "verification": {
+                "code_examined": "code",
+                "line_range_examined": [10, 10],
+                "verification_method": "direct_code_inspection",
+            },
+        }
+        result = ParallelOrchestratorFinding.model_validate(data)
+        assert result.checked_for_handling_elsewhere is False
+
+    def test_checked_for_handling_elsewhere_true(self):
+        """Test checked_for_handling_elsewhere can be set True."""
+        data = {
+            "id": "test-1",
+            "file": "api.py",
+            "line": 25,
+            "title": "Missing error handling",
+            "description": "No try-catch around database call",
+            "category": "quality",
+            "severity": "medium",
+            "checked_for_handling_elsewhere": True,
+            "verification": {
+                "code_examined": "result = db.query(user_input)",
+                "line_range_examined": [25, 25],
+                "verification_method": "cross_file_trace",
+            },
+        }
+        result = ParallelOrchestratorFinding.model_validate(data)
+        assert result.checked_for_handling_elsewhere is True
+
+
+class TestVerificationSchemaGeneration:
+    """Tests for JSON schema generation with VerificationEvidence."""
+
+    def test_verification_in_parallel_orchestrator_schema(self):
+        """Test that VerificationEvidence appears in schema."""
+        schema = ParallelOrchestratorFinding.model_json_schema()
+
+        # verification should be in properties
+        assert "verification" in schema["properties"]
+
+        # Check $defs includes VerificationEvidence
+        assert "$defs" in schema
+        assert "VerificationEvidence" in schema["$defs"]
+
+        # Check VerificationEvidence has correct fields
+        ve_schema = schema["$defs"]["VerificationEvidence"]
+        assert "code_examined" in ve_schema["properties"]
+        assert "line_range_examined" in ve_schema["properties"]
+        assert "verification_method" in ve_schema["properties"]
+
+    def test_new_boolean_fields_in_schema(self):
+        """Test is_impact_finding and checked_for_handling_elsewhere in schema."""
+        schema = ParallelOrchestratorFinding.model_json_schema()
+
+        assert "is_impact_finding" in schema["properties"]
+        assert "checked_for_handling_elsewhere" in schema["properties"]