diff --git a/apps/backend/requirements.txt b/apps/backend/requirements.txt index 13a9e1fb..aa5e2713 100644 --- a/apps/backend/requirements.txt +++ b/apps/backend/requirements.txt @@ -1,5 +1,5 @@ # Auto-Build Framework Dependencies -claude-agent-sdk>=0.1.16 +claude-agent-sdk>=0.1.19 python-dotenv>=1.0.0 # TOML parsing fallback for Python < 3.11 diff --git a/apps/backend/runners/github/context_gatherer.py b/apps/backend/runners/github/context_gatherer.py index 374f11c6..31f5f015 100644 --- a/apps/backend/runners/github/context_gatherer.py +++ b/apps/backend/runners/github/context_gatherer.py @@ -37,6 +37,20 @@ except (ImportError, ValueError, SystemError): SAFE_REF_PATTERN = re.compile(r"^[a-zA-Z0-9._/\-]+$") SAFE_PATH_PATTERN = re.compile(r"^[a-zA-Z0-9._/\-@]+$") +# Common config file names to search for in project directories +# Used by both _find_config_files() and find_related_files_for_root() +CONFIG_FILE_NAMES = [ + "tsconfig.json", + "package.json", + "pyproject.toml", + "setup.py", + ".eslintrc", + ".prettierrc", + "jest.config.js", + "vitest.config.ts", + "vite.config.ts", +] + def _validate_git_ref(ref: str) -> bool: """ @@ -942,20 +956,8 @@ class PRContextGatherer: def _find_config_files(self, directory: Path) -> set[str]: """Find configuration files in a directory.""" - config_names = [ - "tsconfig.json", - "package.json", - "pyproject.toml", - "setup.py", - ".eslintrc", - ".prettierrc", - "jest.config.js", - "vitest.config.ts", - "vite.config.ts", - ] - found = set() - for name in config_names: + for name in CONFIG_FILE_NAMES: config_path = directory / name full_path = self.project_dir / config_path if full_path.exists() and full_path.is_file(): @@ -974,6 +976,68 @@ class PRContextGatherer: return set() + @staticmethod + def find_related_files_for_root( + changed_files: list[ChangedFile], + project_root: Path, + ) -> list[str]: + """ + Find files related to the changes using a specific project root. + + This static method allows finding related files AFTER a worktree + has been created, ensuring files exist in the worktree filesystem. + + Args: + changed_files: List of changed files from the PR + project_root: Path to search for related files (e.g., worktree path) + + Returns: + List of related file paths (relative to project root) + """ + related: set[str] = set() + + for changed_file in changed_files: + path = Path(changed_file.path) + + # Find test files + test_patterns = [ + # Jest/Vitest patterns + path.parent / f"{path.stem}.test{path.suffix}", + path.parent / f"{path.stem}.spec{path.suffix}", + path.parent / "__tests__" / f"{path.name}", + # Python patterns + path.parent / f"test_{path.stem}.py", + path.parent / f"{path.stem}_test.py", + # Go patterns + path.parent / f"{path.stem}_test.go", + ] + + for test_path in test_patterns: + full_path = project_root / test_path + if full_path.exists() and full_path.is_file(): + related.add(str(test_path)) + + # Find config files in same directory + for name in CONFIG_FILE_NAMES: + config_path = path.parent / name + full_path = project_root / config_path + if full_path.exists() and full_path.is_file(): + related.add(str(config_path)) + + # Find type definition files + if path.suffix in [".ts", ".tsx"]: + type_def = path.parent / f"{path.stem}.d.ts" + full_path = project_root / type_def + if full_path.exists() and full_path.is_file(): + related.add(str(type_def)) + + # Remove files that are already in changed_files + changed_paths = {cf.path for cf in changed_files} + related = {r for r in related if r not in changed_paths} + + # Limit to 20 most relevant files + return sorted(related)[:20] + class FollowupContextGatherer: """ diff --git a/apps/backend/runners/github/services/parallel_followup_reviewer.py b/apps/backend/runners/github/services/parallel_followup_reviewer.py index 84e4c8f7..d210801b 100644 --- a/apps/backend/runners/github/services/parallel_followup_reviewer.py +++ b/apps/backend/runners/github/services/parallel_followup_reviewer.py @@ -531,6 +531,7 @@ The SDK will run invoked agents in parallel automatically. stream_result = await process_sdk_stream( client=client, context_name="ParallelFollowup", + model=model, ) # Check for stream processing errors @@ -558,6 +559,17 @@ The SDK will run invoked agents in parallel automatically. if structured_output: result_data = self._parse_structured_output(structured_output, context) else: + # Log when structured output is missing - this shouldn't happen normally + # when output_format is configured, so it indicates a problem + logger.warning( + "[ParallelFollowup] No structured output received from SDK - " + "falling back to text parsing. Resolution data may be incomplete." + ) + safe_print( + "[ParallelFollowup] WARNING: Structured output not captured, " + "using text fallback (resolution tracking may be incomplete)", + flush=True, + ) result_data = self._parse_text_output(result_text, context) # Extract data @@ -623,6 +635,51 @@ The SDK will run invoked agents in parallel automatically. flush=True, ) + # CRITICAL: Enforce CI pending status - cannot approve with pending checks + # This ensures AI compliance with the rule: "Pending CI = NEEDS_REVISION" + ci_status = context.ci_status or {} + pending_ci = ci_status.get("pending", 0) + failing_ci = ci_status.get("failing", 0) + + if failing_ci > 0: + # Failing CI blocks merge + if verdict in ( + MergeVerdict.READY_TO_MERGE, + MergeVerdict.MERGE_WITH_CHANGES, + ): + failed_checks = ci_status.get("failed_checks", []) + checks_str = ( + ", ".join(failed_checks[:3]) if failed_checks else "unknown" + ) + blockers.append( + f"CI Failing: {failing_ci} check(s) failing ({checks_str})" + ) + verdict = MergeVerdict.BLOCKED + verdict_reasoning = ( + f"Blocked: {failing_ci} CI check(s) failing. " + f"Fix CI issues before merge." + ) + safe_print( + f"[ParallelFollowup] ⚠️ CI failing ({failing_ci} checks) - blocking merge", + flush=True, + ) + elif pending_ci > 0: + # Pending CI prevents merge-ready verdicts + if verdict in ( + MergeVerdict.READY_TO_MERGE, + MergeVerdict.MERGE_WITH_CHANGES, + ): + verdict = MergeVerdict.NEEDS_REVISION + verdict_reasoning = ( + f"Ready once CI passes: {pending_ci} check(s) still pending. " + f"All code issues addressed, waiting for CI completion." + ) + safe_print( + f"[ParallelFollowup] ⏳ CI pending ({pending_ci} checks) - " + f"downgrading verdict to NEEDS_REVISION", + flush=True, + ) + for finding in unique_findings: if finding.severity in ( ReviewSeverity.CRITICAL, @@ -928,7 +985,34 @@ The SDK will run invoked agents in parallel automatically. } except Exception as e: + # Log error visibly so users know structured output parsing failed logger.warning(f"[ParallelFollowup] Failed to parse structured output: {e}") + safe_print( + f"[ParallelFollowup] ERROR: Structured output parsing failed: {e}", + flush=True, + ) + safe_print( + "[ParallelFollowup] Attempting to extract partial data from raw output...", + flush=True, + ) + + # Try to extract what we can from the raw dict before giving up + # This handles cases where Pydantic validation fails but data is present + try: + partial_result = self._extract_partial_data(data) + if partial_result: + safe_print( + f"[ParallelFollowup] Recovered partial data: " + f"{len(partial_result.get('resolved_ids', []))} resolved, " + f"{len(partial_result.get('unresolved_ids', []))} unresolved", + flush=True, + ) + return partial_result + except Exception as extract_error: + logger.warning( + f"[ParallelFollowup] Partial extraction also failed: {extract_error}" + ) + return self._create_empty_result() def _parse_text_output(self, text: str, context: FollowupReviewContext) -> dict: @@ -969,6 +1053,75 @@ The SDK will run invoked agents in parallel automatically. "verdict_reasoning": "Unable to parse review results", } + def _extract_partial_data(self, data: dict) -> dict | None: + """ + Extract what data we can from raw output when Pydantic validation fails. + + This handles cases where the AI produced valid data but it doesn't exactly + match the expected schema (missing optional fields, type mismatches, etc.). + """ + if not isinstance(data, dict): + return None + + resolved_ids = [] + unresolved_ids = [] + new_finding_ids = [] + + # Try to extract resolution verifications + resolution_verifications = data.get("resolution_verifications", []) + if isinstance(resolution_verifications, list): + for rv in resolution_verifications: + if isinstance(rv, dict): + finding_id = rv.get("finding_id", "") + status = rv.get("status", "") + if finding_id: + if status == "resolved": + resolved_ids.append(finding_id) + elif status in ( + "unresolved", + "partially_resolved", + "cant_verify", + ): + unresolved_ids.append(finding_id) + + # Try to extract new findings + new_findings = data.get("new_findings", []) + if isinstance(new_findings, list): + for nf in new_findings: + if isinstance(nf, dict): + finding_id = nf.get("id", "") + if finding_id: + new_finding_ids.append(finding_id) + + # Try to extract verdict + verdict_str = data.get("verdict", "NEEDS_REVISION") + verdict_map = { + "READY_TO_MERGE": MergeVerdict.READY_TO_MERGE, + "MERGE_WITH_CHANGES": MergeVerdict.MERGE_WITH_CHANGES, + "NEEDS_REVISION": MergeVerdict.NEEDS_REVISION, + "BLOCKED": MergeVerdict.BLOCKED, + } + verdict = verdict_map.get(verdict_str, MergeVerdict.NEEDS_REVISION) + + verdict_reasoning = data.get("verdict_reasoning", "Extracted from partial data") + + # Only return if we got any useful data + if resolved_ids or unresolved_ids or new_finding_ids: + return { + "findings": [], # Can't reliably extract full findings without validation + "resolved_ids": resolved_ids, + "unresolved_ids": unresolved_ids, + "new_finding_ids": new_finding_ids, + "dismissed_false_positive_ids": [], + "confirmed_valid_count": 0, + "needs_human_review_count": 0, + "verdict": verdict, + "verdict_reasoning": f"[Partial extraction] {verdict_reasoning}", + "agents_invoked": data.get("agents_invoked", []), + } + + return None + def _generate_finding_id(self, file: str, line: int, title: str) -> str: """Generate a unique finding ID.""" content = f"{file}:{line}:{title}" diff --git a/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py b/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py index 1d88a1ff..eef98688 100644 --- a/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py +++ b/apps/backend/runners/github/services/parallel_orchestrator_reviewer.py @@ -28,7 +28,7 @@ from claude_agent_sdk import AgentDefinition try: from ...core.client import create_client from ...phase_config import get_thinking_budget - from ..context_gatherer import PRContext, _validate_git_ref + from ..context_gatherer import PRContext, PRContextGatherer, _validate_git_ref from ..gh_client import GHClient from ..models import ( BRANCH_BEHIND_BLOCKER_MSG, @@ -45,7 +45,7 @@ try: from .pydantic_models import ParallelOrchestratorResponse from .sdk_utils import process_sdk_stream except (ImportError, ValueError, SystemError): - from context_gatherer import PRContext, _validate_git_ref + from context_gatherer import PRContext, PRContextGatherer, _validate_git_ref from core.client import create_client from gh_client import GHClient from models import ( @@ -468,11 +468,9 @@ The SDK will run invoked agents in parallel automatically. pr_number=context.pr_number, ) - # Build orchestrator prompt - prompt = self._build_orchestrator_prompt(context) - # Create temporary worktree at PR head commit for isolated review - # This ensures agents read from the correct PR state, not the current checkout + # This MUST happen BEFORE building the prompt so we can find related files + # that exist in the PR but not in the current checkout head_sha = context.head_sha or context.head_branch if DEBUG_MODE: @@ -518,12 +516,31 @@ The SDK will run invoked agents in parallel automatically. head_sha, context.pr_number ) project_root = worktree_path - if DEBUG_MODE: - safe_print( - f"[PRReview] DEBUG: Using worktree as " - f"project_root={project_root}", - flush=True, - ) + # Count files in worktree to give user visibility (with limit to avoid slowdown) + MAX_FILE_COUNT = 10000 + try: + file_count = 0 + for f in worktree_path.rglob("*"): + if f.is_file() and ".git" not in f.parts: + file_count += 1 + if file_count >= MAX_FILE_COUNT: + break + except (OSError, PermissionError): + file_count = 0 + file_count_str = ( + f"{file_count:,}+" + if file_count >= MAX_FILE_COUNT + else f"{file_count:,}" + ) + # Always log worktree creation with file count (not gated by DEBUG_MODE) + safe_print( + f"[PRReview] Created temporary worktree: {worktree_path.name} ({file_count_str} files)", + flush=True, + ) + safe_print( + f"[PRReview] Worktree contains PR branch HEAD: {head_sha[:8]}", + flush=True, + ) except (RuntimeError, ValueError) as e: if DEBUG_MODE: safe_print( @@ -541,6 +558,29 @@ The SDK will run invoked agents in parallel automatically. else self.project_dir ) + # Rescan for related files using the worktree/project root + # This fixes the issue where related files were 0 because context gathering + # happened BEFORE the worktree was created (PR files didn't exist locally) + if context.changed_files: + new_related_files = PRContextGatherer.find_related_files_for_root( + context.changed_files, + project_root, + ) + # Always log rescan result (not gated by DEBUG_MODE) + if new_related_files: + context.related_files = new_related_files + safe_print( + f"[PRReview] Rescanned in worktree: found {len(new_related_files)} related files" + ) + else: + safe_print( + f"[PRReview] Rescanned in worktree: found 0 related files " + f"(initial scan found {len(context.related_files)})" + ) + + # Build orchestrator prompt AFTER worktree creation and related files rescan + prompt = self._build_orchestrator_prompt(context) + # Use model and thinking level from config (user settings) model = self.config.model or "claude-sonnet-4-5-20250929" thinking_level = self.config.thinking_level or "medium" @@ -575,6 +615,7 @@ The SDK will run invoked agents in parallel automatically. stream_result = await process_sdk_stream( client=client, context_name="ParallelOrchestrator", + model=model, ) # Check for stream processing errors diff --git a/apps/backend/runners/github/services/sdk_utils.py b/apps/backend/runners/github/services/sdk_utils.py index c6ac7d89..f1d21207 100644 --- a/apps/backend/runners/github/services/sdk_utils.py +++ b/apps/backend/runners/github/services/sdk_utils.py @@ -26,6 +26,104 @@ logger = logging.getLogger(__name__) DEBUG_MODE = os.environ.get("DEBUG", "").lower() in ("true", "1", "yes") +def _short_model_name(model: str | None) -> str: + """Convert full model name to a short display name for logs. + + Examples: + claude-sonnet-4-5-20250929 -> sonnet-4.5 + claude-opus-4-5-20251101 -> opus-4.5 + claude-3-5-sonnet-20241022 -> sonnet-3.5 + """ + if not model: + return "unknown" + + model_lower = model.lower() + + # Handle new model naming (claude-{model}-{version}-{date}) + if "opus-4-5" in model_lower or "opus-4.5" in model_lower: + return "opus-4.5" + if "sonnet-4-5" in model_lower or "sonnet-4.5" in model_lower: + return "sonnet-4.5" + if "haiku-4" in model_lower: + return "haiku-4" + + # Handle older model naming (claude-3-5-{model}) + if "3-5-sonnet" in model_lower or "3.5-sonnet" in model_lower: + return "sonnet-3.5" + if "3-5-haiku" in model_lower or "3.5-haiku" in model_lower: + return "haiku-3.5" + if "3-opus" in model_lower: + return "opus-3" + if "3-sonnet" in model_lower: + return "sonnet-3" + if "3-haiku" in model_lower: + return "haiku-3" + + # Fallback: return last part before date (if matches pattern) + parts = model.split("-") + if len(parts) >= 2: + # Try to find model type (opus, sonnet, haiku) + for i, part in enumerate(parts): + if part.lower() in ("opus", "sonnet", "haiku"): + return part.lower() + + return model[:20] # Truncate if nothing else works + + +def _get_tool_detail(tool_name: str, tool_input: dict[str, Any]) -> str: + """Extract meaningful detail from tool input for user-friendly logging. + + Instead of "Using tool: Read", show "Reading sdk_utils.py" + Instead of "Using tool: Grep", show "Searching for 'pattern'" + """ + if tool_name == "Read": + file_path = tool_input.get("file_path", "") + if file_path: + # Extract just the filename for brevity + filename = file_path.split("/")[-1] if "/" in file_path else file_path + return f"Reading {filename}" + return "Reading file" + + if tool_name == "Grep": + pattern = tool_input.get("pattern", "") + if pattern: + # Truncate long patterns + pattern_preview = pattern[:40] + "..." if len(pattern) > 40 else pattern + return f"Searching for '{pattern_preview}'" + return "Searching codebase" + + if tool_name == "Glob": + pattern = tool_input.get("pattern", "") + if pattern: + return f"Finding files matching '{pattern}'" + return "Finding files" + + if tool_name == "Bash": + command = tool_input.get("command", "") + if command: + # Show first part of command + cmd_preview = command[:50] + "..." if len(command) > 50 else command + return f"Running: {cmd_preview}" + return "Running command" + + if tool_name == "Edit": + file_path = tool_input.get("file_path", "") + if file_path: + filename = file_path.split("/")[-1] if "/" in file_path else file_path + return f"Editing {filename}" + return "Editing file" + + if tool_name == "Write": + file_path = tool_input.get("file_path", "") + if file_path: + filename = file_path.split("/")[-1] if "/" in file_path else file_path + return f"Writing {filename}" + return "Writing file" + + # Default fallback for unknown tools + return f"Using tool: {tool_name}" + + async def process_sdk_stream( client: Any, on_thinking: Callable[[str], None] | None = None, @@ -34,6 +132,7 @@ async def process_sdk_stream( on_text: Callable[[str], None] | None = None, on_structured_output: Callable[[dict[str, Any]], None] | None = None, context_name: str = "SDK", + model: str | None = None, ) -> dict[str, Any]: """ Process SDK response stream with customizable callbacks. @@ -43,7 +142,7 @@ async def process_sdk_stream( - Tracking tool invocations (especially Task/subagent calls) - Tracking tool results - Collecting text output - - Extracting structured output + - Extracting structured output (per official Python SDK pattern) Args: client: Claude SDK client with receive_response() method @@ -53,6 +152,7 @@ async def process_sdk_stream( on_text: Callback for text output - receives text string 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") Returns: Dictionary with: @@ -70,17 +170,40 @@ async def process_sdk_stream( stream_error = None # Track subagent tool IDs to log their results subagent_tool_ids: dict[str, str] = {} # tool_id -> agent_name + completed_agent_tool_ids: set[str] = set() # tool_ids of completed agents safe_print(f"[{context_name}] Processing SDK stream...") if DEBUG_MODE: safe_print(f"[DEBUG {context_name}] Awaiting response stream...") + # Track activity for progress logging + last_progress_log = 0 + PROGRESS_LOG_INTERVAL = 10 # Log progress every N messages + try: async for msg in client.receive_response(): try: msg_type = type(msg).__name__ msg_count += 1 + # Log progress periodically so user knows AI is working + if msg_count - last_progress_log >= PROGRESS_LOG_INTERVAL: + if subagent_tool_ids: + pending = len(subagent_tool_ids) - len(completed_agent_tool_ids) + if pending > 0: + safe_print( + f"[{context_name}] Processing... ({msg_count} messages, {pending} agent{'s' if pending > 1 else ''} working)" + ) + else: + safe_print( + f"[{context_name}] Processing... ({msg_count} messages)" + ) + else: + safe_print( + f"[{context_name}] Processing... ({msg_count} messages)" + ) + last_progress_log = msg_count + if DEBUG_MODE: # Log every message type for visibility msg_details = "" @@ -130,23 +253,15 @@ 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 - safe_print(f"[{context_name}] Invoked agent: {agent_name}") - elif tool_name == "StructuredOutput": - if tool_input: - # Warn if overwriting existing structured output - if structured_output is not None: - logger.warning( - f"[{context_name}] Multiple StructuredOutput blocks received, " - f"overwriting previous output" - ) - structured_output = tool_input - safe_print(f"[{context_name}] Received structured output") - # Invoke callback - if on_structured_output: - on_structured_output(tool_input) - elif DEBUG_MODE: - # Log other tool calls in debug mode - safe_print(f"[DEBUG {context_name}] Other tool: {tool_name}") + # 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}" + ) + elif tool_name != "StructuredOutput": + # Log meaningful tool info (not just tool name) + tool_detail = _get_tool_detail(tool_name, tool_input) + safe_print(f"[{context_name}] {tool_detail}") # Invoke callback for all tool uses if on_tool_use: @@ -169,6 +284,7 @@ async def process_sdk_stream( # Check if this is a subagent result if tool_id in subagent_tool_ids: agent_name = subagent_tool_ids[tool_id] + completed_agent_tool_ids.add(tool_id) # Mark agent as completed status = "ERROR" if is_error else "complete" result_preview = ( str(result_content)[:600].replace("\n", " ").strip() @@ -176,11 +292,17 @@ async def process_sdk_stream( safe_print( f"[Agent:{agent_name}] {status}: {result_preview}{'...' if len(str(result_content)) > 600 else ''}" ) - elif DEBUG_MODE: - status = "ERROR" if is_error else "OK" - safe_print( - f"[DEBUG {context_name}] Tool result: {tool_id} [{status}]" + else: + # Show tool completion for visibility (not gated by DEBUG) + status = "ERROR" if is_error else "done" + # Show brief preview of result for context + result_preview = ( + str(result_content)[:100].replace("\n", " ").strip() ) + if result_preview: + safe_print( + f"[{context_name}] Tool result [{status}]: {result_preview}{'...' if len(str(result_content)) > 100 else ''}" + ) # Invoke callback if on_tool_result: @@ -205,21 +327,19 @@ async def process_sdk_stream( if agent_name not in agents_invoked: agents_invoked.append(agent_name) subagent_tool_ids[tool_id] = agent_name - safe_print( - f"[{context_name}] Invoking agent: {agent_name}" + # Log with model info if available + model_info = ( + f" [{_short_model_name(model)}]" + if model + else "" ) - elif tool_name == "StructuredOutput": - if tool_input: - # Warn if overwriting existing structured output - if structured_output is not None: - logger.warning( - f"[{context_name}] Multiple StructuredOutput blocks received, " - f"overwriting previous output" - ) - structured_output = tool_input - # Invoke callback - if on_structured_output: - on_structured_output(tool_input) + safe_print( + f"[{context_name}] Invoking agent: {agent_name}{model_info}" + ) + elif tool_name != "StructuredOutput": + # Log meaningful tool info (not just tool name) + tool_detail = _get_tool_detail(tool_name, tool_input) + safe_print(f"[{context_name}] {tool_detail}") # Invoke callback if on_tool_use: @@ -239,33 +359,48 @@ async def process_sdk_stream( if on_text: on_text(block.text) - # Check for StructuredOutput in content (legacy check) - if getattr(block, "name", "") == "StructuredOutput": - structured_data = getattr(block, "input", None) - if structured_data: - # Warn if overwriting existing structured output - if structured_output is not None: - logger.warning( - f"[{context_name}] Multiple StructuredOutput blocks received, " - f"overwriting previous output" - ) - structured_output = structured_data - # Invoke callback - if on_structured_output: - on_structured_output(structured_data) + # ================================================================ + # STRUCTURED OUTPUT CAPTURE (Single, consolidated location) + # Per official Python SDK docs: https://platform.claude.com/docs/en/agent-sdk/structured-outputs + # The Python pattern is: if hasattr(message, 'structured_output') + # ================================================================ - # Check for structured_output attribute - if hasattr(msg, "structured_output") and msg.structured_output: - # Warn if overwriting existing structured output - if structured_output is not None: - logger.warning( - f"[{context_name}] Multiple StructuredOutput blocks received, " - f"overwriting previous output" + # Check for error_max_structured_output_retries first (SDK validation failed) + is_result_msg = msg_type == "ResultMessage" or ( + hasattr(msg, "type") and msg.type == "result" + ) + if is_result_msg: + subtype = getattr(msg, "subtype", None) + if DEBUG_MODE: + safe_print( + f"[DEBUG {context_name}] ResultMessage: subtype={subtype}" + ) + if subtype == "error_max_structured_output_retries": + # SDK failed to produce valid structured output after retries + logger.warning( + f"[{context_name}] Claude could not produce valid structured output " + f"after maximum retries - schema validation failed" + ) + safe_print( + f"[{context_name}] WARNING: Structured output validation failed after retries" + ) + if not stream_error: + stream_error = "structured_output_validation_failed" + + # Capture structured output from ANY message that has it + # This is the official Python SDK pattern - check hasattr() + if hasattr(msg, "structured_output") and msg.structured_output: + # Only capture if we don't already have it (avoid duplicates) + if structured_output is None: + structured_output = msg.structured_output + safe_print(f"[{context_name}] Received structured output") + if on_structured_output: + on_structured_output(msg.structured_output) + elif DEBUG_MODE: + # In debug mode, note that we skipped a duplicate + safe_print( + f"[DEBUG {context_name}] Skipping duplicate structured output" ) - structured_output = msg.structured_output - # Invoke callback - if on_structured_output: - on_structured_output(msg.structured_output) # Check for tool results in UserMessage (subagent results come back here) if msg_type == "UserMessage" and hasattr(msg, "content"): @@ -289,6 +424,9 @@ async def process_sdk_stream( # Check if this is a subagent result if tool_id in subagent_tool_ids: agent_name = subagent_tool_ids[tool_id] + completed_agent_tool_ids.add( + tool_id + ) # Mark agent as completed status = "ERROR" if is_error else "complete" result_preview = ( str(result_content)[:600].replace("\n", " ").strip() diff --git a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts index d1dacecf..d2c7be05 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -544,15 +544,21 @@ function parseLogLine(line: string): { source: string; content: string; isError: * Determine the phase from source */ function getPhaseFromSource(source: string): PRLogPhase { - const contextSources = ["Context", "BotDetector"]; + // Context phase: gathering PR data, commits, files, feedback + // Note: "Followup" is context gathering for follow-up reviews (comparing commits, finding changes) + const contextSources = ["Context", "BotDetector", "Followup"]; + // Analysis phase: AI agents analyzing code const analysisSources = [ "AI", "Orchestrator", "ParallelOrchestrator", "ParallelFollowup", - "Followup", "orchestrator", + "PRReview", // Worktree creation and PR-specific analysis + "ClientCache", // SDK client cache operations ]; + // Synthesis phase: final summary and results + // Note: "Progress" logs are redundant (shown in progress bar) but kept for completeness const synthesisSources = ["PR Review Engine", "Summary", "Progress"]; if (contextSources.includes(source)) return "context"; diff --git a/apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx b/apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx index d7aa3936..c5d5f02c 100644 --- a/apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx +++ b/apps/frontend/src/renderer/components/github-prs/components/PRLogs.tsx @@ -1,4 +1,5 @@ import { useState } from 'react'; +import { useTranslation } from 'react-i18next'; import { Terminal, Loader2, @@ -10,7 +11,8 @@ import { ChevronDown, ChevronRight, Info, - Clock + Clock, + Activity } from 'lucide-react'; import { Badge } from '../../ui/badge'; import { Collapsible, CollapsibleTrigger, CollapsibleContent } from '../../ui/collapsible'; @@ -71,8 +73,72 @@ const SOURCE_COLORS: Record = { 'default': 'bg-muted text-muted-foreground' }; +// Helper type for grouped agent entries +interface AgentGroup { + agentName: string; + entries: PRLogEntry[]; +} + +// Patterns that indicate orchestrator tool activity (vs. important messages) +const TOOL_ACTIVITY_PATTERNS = [ + /^Reading /, + /^Searching for /, + /^Finding files /, + /^Running: /, + /^Editing /, + /^Writing /, + /^Using tool: /, + /^Processing\.\.\. \(\d+ messages/, + /^Tool result \[/, +]; + +function isToolActivityLog(content: string): boolean { + return TOOL_ACTIVITY_PATTERNS.some(pattern => pattern.test(content)); +} + +// Group entries by: agents, orchestrator activity, and other entries +function groupEntriesByAgent(entries: PRLogEntry[]): { + agentGroups: AgentGroup[]; + orchestratorActivity: PRLogEntry[]; + otherEntries: PRLogEntry[]; +} { + const agentMap = new Map(); + const orchestratorActivity: PRLogEntry[] = []; + const otherEntries: PRLogEntry[] = []; + + for (const entry of entries) { + if (entry.source?.startsWith('Agent:')) { + // Agent results + const existing = agentMap.get(entry.source) || []; + existing.push(entry); + agentMap.set(entry.source, existing); + } else if ( + (entry.source === 'ParallelOrchestrator' || entry.source === 'ParallelFollowup') && + isToolActivityLog(entry.content) + ) { + // Orchestrator tool activity (verbose logs) + orchestratorActivity.push(entry); + } else { + // Important messages (AI response, Invoking agent, etc.) + otherEntries.push(entry); + } + } + + // Convert map to array of groups, sorted by first entry timestamp + const agentGroups: AgentGroup[] = Array.from(agentMap.entries()) + .map(([agentName, agentEntries]) => ({ agentName, entries: agentEntries })) + .sort((a, b) => { + const aTime = a.entries[0]?.timestamp || ''; + const bTime = b.entries[0]?.timestamp || ''; + return aTime.localeCompare(bTime); + }); + + return { agentGroups, orchestratorActivity, otherEntries }; +} + export function PRLogs({ prNumber, logs, isLoading, isStreaming = false }: PRLogsProps) { const [expandedPhases, setExpandedPhases] = useState>(new Set(['analysis'])); + const [expandedAgents, setExpandedAgents] = useState>(new Set()); const togglePhase = (phase: PRLogPhase) => { setExpandedPhases(prev => { @@ -86,6 +152,18 @@ export function PRLogs({ prNumber, logs, isLoading, isStreaming = false }: PRLog }); }; + const toggleAgent = (agentKey: string) => { + setExpandedAgents(prev => { + const next = new Set(prev); + if (next.has(agentKey)) { + next.delete(agentKey); + } else { + next.add(agentKey); + } + return next; + }); + }; + return (
@@ -122,6 +200,8 @@ export function PRLogs({ prNumber, logs, isLoading, isStreaming = false }: PRLog isExpanded={expandedPhases.has(phase)} onToggle={() => togglePhase(phase)} isStreaming={isStreaming} + expandedAgents={expandedAgents} + onToggleAgent={toggleAgent} /> ))} @@ -150,9 +230,11 @@ interface PhaseLogSectionProps { isExpanded: boolean; onToggle: () => void; isStreaming?: boolean; + expandedAgents: Set; + onToggleAgent: (agentKey: string) => void; } -function PhaseLogSection({ phase, phaseLog, isExpanded, onToggle, isStreaming = false }: PhaseLogSectionProps) { +function PhaseLogSection({ phase, phaseLog, isExpanded, onToggle, isStreaming = false, expandedAgents, onToggleAgent }: PhaseLogSectionProps) { const Icon = PHASE_ICONS[phase]; const status = phaseLog?.status || 'pending'; const hasEntries = (phaseLog?.entries.length || 0) > 0; @@ -235,13 +317,16 @@ function PhaseLogSection({ phase, phaseLog, isExpanded, onToggle, isStreaming = -
+
{!hasEntries ? (

No logs yet

) : ( - phaseLog?.entries.map((entry, idx) => ( - - )) + )}
@@ -249,6 +334,191 @@ function PhaseLogSection({ phase, phaseLog, isExpanded, onToggle, isStreaming = ); } +// Grouped Log Entries Component - renders agents grouped with collapsible sections +interface GroupedLogEntriesProps { + entries: PRLogEntry[]; + phase: PRLogPhase; + expandedAgents: Set; + onToggleAgent: (agentKey: string) => void; +} + +function GroupedLogEntries({ entries, phase, expandedAgents, onToggleAgent }: GroupedLogEntriesProps) { + const { agentGroups, orchestratorActivity, otherEntries } = groupEntriesByAgent(entries); + + return ( +
+ {/* Render important messages first (AI response, Invoking agent, etc.) */} + {otherEntries.length > 0 && ( +
+ {otherEntries.map((entry, idx) => ( + + ))} +
+ )} + + {/* Render orchestrator tool activity in collapsible section */} + {orchestratorActivity.length > 0 && ( + onToggleAgent(`${phase}-orchestrator-activity`)} + /> + )} + + {/* Render agent groups with collapsible sections */} + {agentGroups.map((group) => ( + onToggleAgent(`${phase}-${group.agentName}`)} + /> + ))} +
+ ); +} + +// Orchestrator Activity Section - collapsible section for tool activity logs +interface OrchestratorActivitySectionProps { + entries: PRLogEntry[]; + phase: PRLogPhase; + isExpanded: boolean; + onToggle: () => void; +} + +function OrchestratorActivitySection({ entries, isExpanded, onToggle }: OrchestratorActivitySectionProps) { + const { t } = useTranslation(['common']); + + // Count different types of operations for summary + const readCount = entries.filter(e => e.content.startsWith('Reading ')).length; + const searchCount = entries.filter(e => e.content.startsWith('Searching for ')).length; + const otherCount = entries.length - readCount - searchCount; + + // Build summary text + const summaryParts: string[] = []; + if (readCount > 0) summaryParts.push(`${readCount} file${readCount > 1 ? 's' : ''} read`); + if (searchCount > 0) summaryParts.push(`${searchCount} search${searchCount > 1 ? 'es' : ''}`); + if (otherCount > 0) summaryParts.push(`${otherCount} other`); + const summary = summaryParts.join(', ') || `${entries.length} operations`; + + return ( +
+ + + {isExpanded && ( +
+ {entries.map((entry, idx) => ( +
+ + {new Date(entry.timestamp).toLocaleTimeString('en-US', { hour: '2-digit', minute: '2-digit', second: '2-digit' })} + + {entry.content} +
+ ))} +
+ )} +
+ ); +} + +// Agent Log Group Component - shows first message + expandable section for more +interface AgentLogGroupProps { + group: AgentGroup; + phase: PRLogPhase; + isExpanded: boolean; + onToggle: () => void; +} + +function AgentLogGroup({ group, isExpanded, onToggle }: AgentLogGroupProps) { + const { t } = useTranslation(['common']); + const { agentName, entries } = group; + const hasMultipleEntries = entries.length > 1; + const firstEntry = entries[0]; + const remainingEntries = entries.slice(1); + + // Extract display name from "Agent:logic-reviewer" -> "logic-reviewer" + const displayName = agentName.replace('Agent:', ''); + + const getSourceColor = (source: string) => { + return SOURCE_COLORS[source] || SOURCE_COLORS.default; + }; + + return ( +
+ {/* Agent header with first message always visible */} +
+ {/* Agent badge header */} +
+ + {displayName} + + {hasMultipleEntries && ( + + )} +
+ + {/* First entry (summary) - always visible */} + {firstEntry && ( + + )} +
+ + {/* Collapsible section for remaining entries */} + {hasMultipleEntries && isExpanded && ( +
+ {remainingEntries.map((entry, idx) => ( + + ))} +
+ )} +
+ ); +} + // Log Entry Component interface LogEntryProps { entry: PRLogEntry; diff --git a/apps/frontend/src/shared/i18n/locales/en/common.json b/apps/frontend/src/shared/i18n/locales/en/common.json index aa11f3e2..907cd30c 100644 --- a/apps/frontend/src/shared/i18n/locales/en/common.json +++ b/apps/frontend/src/shared/i18n/locales/en/common.json @@ -338,7 +338,12 @@ "cleanReviewMessageFooter": "*This automated review found no issues. Generated by Auto Claude.*", "failedPostCleanReview": "Failed to post clean review", "viewErrorDetails": "View details", - "hideErrorDetails": "Hide details" + "hideErrorDetails": "Hide details", + "logs": { + "agentActivity": "Agent Activity", + "showMore": "Show {{count}} more", + "hideMore": "Hide {{count}} more" + } }, "downloads": { "toggleExpand": "Toggle download details", diff --git a/apps/frontend/src/shared/i18n/locales/fr/common.json b/apps/frontend/src/shared/i18n/locales/fr/common.json index 3861d444..f27975fa 100644 --- a/apps/frontend/src/shared/i18n/locales/fr/common.json +++ b/apps/frontend/src/shared/i18n/locales/fr/common.json @@ -338,7 +338,12 @@ "cleanReviewMessageFooter": "*This automated review found no issues. Generated by Auto Claude.*", "failedPostCleanReview": "Échec de la publication de la révision", "viewErrorDetails": "Voir les détails", - "hideErrorDetails": "Masquer les détails" + "hideErrorDetails": "Masquer les détails", + "logs": { + "agentActivity": "Activité des agents", + "showMore": "Afficher {{count}} de plus", + "hideMore": "Masquer {{count}}" + } }, "downloads": { "toggleExpand": "Afficher/masquer les détails", diff --git a/tests/test_dependency_validator.py b/tests/test_dependency_validator.py index c0e42275..550eb7b2 100644 --- a/tests/test_dependency_validator.py +++ b/tests/test_dependency_validator.py @@ -402,7 +402,8 @@ class TestCliUtilsGetProjectDir: from cli.utils import get_project_dir result = get_project_dir(temp_dir) - assert result == temp_dir + # Resolve symlinks for comparison (macOS /var -> /private/var) + assert result.resolve() == temp_dir.resolve() def test_get_project_dir_auto_detects_backend(self, temp_dir): """Auto-detect when running from apps/backend directory.""" @@ -420,7 +421,8 @@ class TestCliUtilsGetProjectDir: os.chdir(backend_dir) result = get_project_dir(None) # Should go up 2 levels from backend to project root - assert result == temp_dir + # Resolve symlinks for comparison (macOS /var -> /private/var) + assert result.resolve() == temp_dir.resolve() finally: os.chdir(original_cwd) @@ -442,8 +444,9 @@ class TestCliUtilsSetupEnvironment: script_dir = setup_environment() # Verify script_dir is the apps/backend directory - assert script_dir.name == "backend" - assert script_dir.parent.name == "apps" + # Use case-insensitive comparison for macOS filesystem compatibility + assert script_dir.name.lower() == "backend" + assert script_dir.parent.name.lower() == "apps" def test_setup_environment_adds_to_path(self): """Add script directory to sys.path."""