fix(pr-review): properly capture structured output from SDK ResultMessage (#1133)
* fix(pr-review): properly capture structured output from SDK ResultMessage Fixes critical bug where PR follow-up reviews showed "0 previous findings addressed" despite AI correctly analyzing resolution status. Root Causes Fixed: 1. sdk_utils.py - ResultMessage handling - Added proper check for msg.type == "result" per Anthropic SDK docs - Handle msg.subtype == "success" for structured output capture - Handle error_max_structured_output_retries error case - Added visible logging when structured output is captured 2. parallel_followup_reviewer.py - Silent fallback prevention - Added warning logging when structured output is missing - Added _extract_partial_data() to recover data when Pydantic fails - Prevents complete data loss when schema validation has minor issues 3. parallel_followup_reviewer.py - CI status enforcement - Added code enforcement for failing CI (override to BLOCKED) - Added enforcement for pending CI (downgrade READY_TO_MERGE) - AI prompt compliance is no longer the only safeguard 4. test_dependency_validator.py - macOS compatibility fixes - Fixed symlink comparison issue (/var vs /private/var) - Fixed case-sensitivity comparison for filesystem Impact: Before: AI analysis showed "3/4 resolved" but summary showed "0 resolved" After: Structured output properly captured, fallback extraction if needed Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(pr-review): rescan related files after worktree creation Related files were always returning 0 because context gathering happened BEFORE the worktree was created. For fork PRs or PRs with new files, the files don't exist in the local checkout, so the related files lookup failed. This fix: - Adds `find_related_files_for_root()` static method to ContextGatherer that can search for related files using any project root path - Restructures ParallelOrchestratorReviewer.review() to create the worktree FIRST, then rescan for related files using the worktree path, then build the prompt with the updated context Now the PR review will correctly find related test files, config files, and type definitions that exist in the PR. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(pr-review): add visible logging for worktree creation and rescan Add always-visible logs (not gated by DEBUG_MODE) to show: - When worktree is created for PR review - Result of related files rescan in worktree This helps verify the fix is working and diagnose issues. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(pr-review): show model name when invoking specialist agents Add model information to agent invocation logs so users can see which model each agent is using. This helps with debugging and monitoring. Example log output: [ParallelOrchestrator] Invoking agent: logic-reviewer [sonnet-4.5] [ParallelOrchestrator] Invoking agent: quality-reviewer [sonnet-4.5] Added _short_model_name() helper to convert full model names like "claude-sonnet-4-5-20250929" to short display names like "sonnet-4.5". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(sdk-utils): add model info to AssistantMessage tool invocation logs The agent invocation log was missing the model info when tool calls came through AssistantMessage content blocks (vs standalone ToolUseBlock). Now both code paths show the model name consistently. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(sdk-utils): add user-visible progress and activity logging Previously, most SDK stream activity was hidden behind DEBUG_MODE, making it hard for users to see what's happening during PR reviews. Changes: - Add periodic progress logs every 10 messages showing agent count - Show tool usage (Read, Grep, etc.) not just Task calls - Show tool completion results with brief preview - Model info now shown for all agent invocation paths Users will now see: - "[ParallelOrchestrator] Processing... (20 messages, 4 agents working)" - "[ParallelOrchestrator] Using tool: Read" - "[ParallelOrchestrator] Tool result [done]: ..." - "[ParallelOrchestrator] Invoking agent: logic-reviewer [opus-4.5]" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(pr-review): improve worktree visibility and fix log categorization 1. Frontend log categorization: - Add "PRReview" and "ClientCache" to analysisSources - [PRReview] logs now appear in "AI Analysis" section instead of "Synthesis" 2. Enhanced worktree logging: - Show file count in worktree creation log - Display PR branch HEAD SHA for verification - Format: "[PRReview] Created temporary worktree: pr-xxx (1,234 files)" 3. Structured output detection: - Also check for msg_type == "ResultMessage" (SDK class name) - Add diagnostic logging in DEBUG mode to trace ResultMessage handling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore(deps): update claude-agent-sdk to >=0.1.19 Update to latest SDK version for structured output improvements. Previous: >=0.1.16 Latest available: 0.1.19 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(sdk-utils): consolidate structured output capture to single location BREAKING: Simplified structured output handling to follow official Python SDK pattern. Before: 5 different capture locations causing "Multiple StructuredOutput blocks" warnings After: 1 capture location using hasattr(msg, 'structured_output') per official docs Changes: - Remove 4 redundant capture paths (ToolUseBlock, AssistantMessage content, legacy, ResultMessage) - Single capture point: if hasattr(msg, 'structured_output') and msg.structured_output - Skip duplicates silently (only capture first one) - Keep error handling for error_max_structured_output_retries - Skip logging StructuredOutput tool calls (handled separately) - Cleaner, more maintainable code following official SDK pattern Reference: https://platform.claude.com/docs/en/agent-sdk/structured-outputs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(pr-logs): enhance log visibility and organization for agent activities - Introduced a new logging structure to categorize agent logs into groups, improving readability and user experience. - Added functionality to toggle visibility of agent logs and orchestrator tool activities, allowing users to focus on relevant information. - Implemented helper functions to identify tool activity logs and group entries by agent, enhancing log organization. - Updated UI components to support the new log grouping and toggling features, ensuring a seamless user interface. This update aims to provide clearer insights into agent activities during PR reviews, making it easier for users to track progress and actions taken by agents. * fix(pr-review): address PR review findings for reliability and UX - Fix CI pending check asymmetry: check MERGE_WITH_CHANGES verdict - Add file count limit (10k) to prevent slow rglob on large repos - Extract CONFIG_FILE_NAMES constant to fix DRY violation - Fix misleading "agents working" count by tracking completed agents - Add i18n translations for agent activity logs (en/fr) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(pr-logs): categorize Followup logs to context phase for follow-up reviews The Followup source logs context gathering work (comparing commits, finding changed files, gathering feedback) not analysis. Move from analysisSources to contextSources so follow-up review logs appear in the correct phase. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Andy
@@ -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
|
||||
|
||||
@@ -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:
|
||||
"""
|
||||
|
||||
@@ -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}"
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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<string, string> = {
|
||||
'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<string, PRLogEntry[]>();
|
||||
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<Set<PRLogPhase>>(new Set(['analysis']));
|
||||
const [expandedAgents, setExpandedAgents] = useState<Set<string>>(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 (
|
||||
<div className="h-full overflow-y-auto scrollbar-thin scrollbar-thumb-border scrollbar-track-transparent">
|
||||
<div className="p-4 space-y-2">
|
||||
@@ -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<string>;
|
||||
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 =
|
||||
</button>
|
||||
</CollapsibleTrigger>
|
||||
<CollapsibleContent>
|
||||
<div className="mt-1 ml-6 border-l-2 border-border pl-4 py-2 space-y-1">
|
||||
<div className="mt-1 ml-6 border-l-2 border-border pl-4 py-2 space-y-2">
|
||||
{!hasEntries ? (
|
||||
<p className="text-xs text-muted-foreground italic">No logs yet</p>
|
||||
) : (
|
||||
phaseLog?.entries.map((entry, idx) => (
|
||||
<LogEntry key={`${entry.timestamp}-${idx}`} entry={entry} />
|
||||
))
|
||||
<GroupedLogEntries
|
||||
entries={phaseLog?.entries || []}
|
||||
phase={phase}
|
||||
expandedAgents={expandedAgents}
|
||||
onToggleAgent={onToggleAgent}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
</CollapsibleContent>
|
||||
@@ -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<string>;
|
||||
onToggleAgent: (agentKey: string) => void;
|
||||
}
|
||||
|
||||
function GroupedLogEntries({ entries, phase, expandedAgents, onToggleAgent }: GroupedLogEntriesProps) {
|
||||
const { agentGroups, orchestratorActivity, otherEntries } = groupEntriesByAgent(entries);
|
||||
|
||||
return (
|
||||
<div className="space-y-2">
|
||||
{/* Render important messages first (AI response, Invoking agent, etc.) */}
|
||||
{otherEntries.length > 0 && (
|
||||
<div className="space-y-1">
|
||||
{otherEntries.map((entry, idx) => (
|
||||
<LogEntry key={`other-${entry.timestamp}-${idx}`} entry={entry} />
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Render orchestrator tool activity in collapsible section */}
|
||||
{orchestratorActivity.length > 0 && (
|
||||
<OrchestratorActivitySection
|
||||
entries={orchestratorActivity}
|
||||
phase={phase}
|
||||
isExpanded={expandedAgents.has(`${phase}-orchestrator-activity`)}
|
||||
onToggle={() => onToggleAgent(`${phase}-orchestrator-activity`)}
|
||||
/>
|
||||
)}
|
||||
|
||||
{/* Render agent groups with collapsible sections */}
|
||||
{agentGroups.map((group) => (
|
||||
<AgentLogGroup
|
||||
key={`${phase}-${group.agentName}`}
|
||||
group={group}
|
||||
phase={phase}
|
||||
isExpanded={expandedAgents.has(`${phase}-${group.agentName}`)}
|
||||
onToggle={() => onToggleAgent(`${phase}-${group.agentName}`)}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// 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 (
|
||||
<div className="rounded-md border border-border/50 bg-secondary/10 overflow-hidden">
|
||||
<button
|
||||
onClick={onToggle}
|
||||
className={cn(
|
||||
'w-full flex items-center justify-between p-2 transition-colors',
|
||||
'hover:bg-secondary/30',
|
||||
isExpanded && 'bg-secondary/20'
|
||||
)}
|
||||
>
|
||||
<div className="flex items-center gap-2">
|
||||
{isExpanded ? (
|
||||
<ChevronDown className="h-3 w-3 text-muted-foreground" />
|
||||
) : (
|
||||
<ChevronRight className="h-3 w-3 text-muted-foreground" />
|
||||
)}
|
||||
<Activity className="h-3 w-3 text-orange-400" />
|
||||
<span className="text-xs text-muted-foreground">{t('common:prReview.logs.agentActivity')}</span>
|
||||
</div>
|
||||
<Badge variant="outline" className="text-[9px] px-1.5 py-0 bg-orange-500/10 text-orange-400 border-orange-500/30">
|
||||
{summary}
|
||||
</Badge>
|
||||
</button>
|
||||
|
||||
{isExpanded && (
|
||||
<div className="border-t border-border/30 p-2 space-y-0.5 max-h-[300px] overflow-y-auto">
|
||||
{entries.map((entry, idx) => (
|
||||
<div key={`activity-${entry.timestamp}-${idx}`} className="flex items-start gap-2 text-[10px] text-muted-foreground/80 py-0.5">
|
||||
<span className="text-muted-foreground/50 tabular-nums shrink-0">
|
||||
{new Date(entry.timestamp).toLocaleTimeString('en-US', { hour: '2-digit', minute: '2-digit', second: '2-digit' })}
|
||||
</span>
|
||||
<span className="break-words">{entry.content}</span>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// 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 (
|
||||
<div className="rounded-md border border-border/50 bg-secondary/20 overflow-hidden">
|
||||
{/* Agent header with first message always visible */}
|
||||
<div className="p-2 space-y-1">
|
||||
{/* Agent badge header */}
|
||||
<div className="flex items-center justify-between">
|
||||
<Badge
|
||||
variant="outline"
|
||||
className={cn('text-[10px] px-1.5 py-0.5', getSourceColor(agentName))}
|
||||
>
|
||||
{displayName}
|
||||
</Badge>
|
||||
{hasMultipleEntries && (
|
||||
<button
|
||||
onClick={onToggle}
|
||||
className={cn(
|
||||
'flex items-center gap-1 text-[10px] px-2 py-0.5 rounded transition-colors',
|
||||
'text-muted-foreground hover:text-foreground hover:bg-secondary/50',
|
||||
isExpanded && 'bg-secondary/50 text-foreground'
|
||||
)}
|
||||
>
|
||||
{isExpanded ? (
|
||||
<>
|
||||
<ChevronDown className="h-3 w-3" />
|
||||
<span>{t('common:prReview.logs.hideMore', { count: remainingEntries.length })}</span>
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<ChevronRight className="h-3 w-3" />
|
||||
<span>{t('common:prReview.logs.showMore', { count: remainingEntries.length })}</span>
|
||||
</>
|
||||
)}
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* First entry (summary) - always visible */}
|
||||
{firstEntry && (
|
||||
<LogEntry entry={{ ...firstEntry, source: undefined }} />
|
||||
)}
|
||||
</div>
|
||||
|
||||
{/* Collapsible section for remaining entries */}
|
||||
{hasMultipleEntries && isExpanded && (
|
||||
<div className="border-t border-border/30 bg-secondary/10 p-2 space-y-1">
|
||||
{remainingEntries.map((entry, idx) => (
|
||||
<LogEntry key={`${entry.timestamp}-${idx}`} entry={{ ...entry, source: undefined }} />
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// Log Entry Component
|
||||
interface LogEntryProps {
|
||||
entry: PRLogEntry;
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user