agent-review

What this skill does

# Agent Implementation Review

Review AI agent implementations for architectural best practices.

**Target:** $ARGUMENTS (path to agent project or codebase)

## When to Use This Skill

- Auditing existing agent implementations
- Designing new agent architectures
- Reviewing agent code for production readiness
- Evaluating multi-agent system designs
- Assessing agent reliability and observability

## Review Process

1. **Discover** - Explore folder structure at $ARGUMENTS
2. **Analyze** - Check against architecture patterns
3. **Evaluate** - Score each category
4. **Report** - Generate findings with recommendations

## Folder Structure Best Practices

### Recommended Agent Project Structure

```
agent-project/
├── src/
│   ├── agents/              # Agent definitions
│   │   ├── base.py          # Base agent class
│   │   ├── planner.py       # Planning agent
│   │   └── executor.py      # Execution agent
│   ├── tools/               # Tool implementations
│   │   ├── __init__.py
│   │   ├── base.py          # Tool base class/interface
│   │   ├── search.py        # Search tool
│   │   └── code.py          # Code execution tool
│   ├── memory/              # Memory/state management
│   │   ├── short_term.py    # Conversation context
│   │   ├── long_term.py     # Persistent storage
│   │   └── vector_store.py  # Embeddings/RAG
│   ├── prompts/             # Prompt templates
│   │   ├── system.py        # System prompts
│   │   └── templates/       # Jinja/string templates
│   ├── orchestration/       # Multi-agent coordination
│   │   ├── router.py        # Request routing
│   │   └── workflow.py      # Agent workflows
│   ├── models/              # Data models/schemas
│   │   ├── messages.py      # Message types
│   │   └── state.py         # State schemas
│   └── utils/               # Shared utilities
│       ├── logging.py       # Structured logging
│       └── retry.py         # Retry logic
├── config/                  # Configuration
│   ├── default.yaml         # Default settings
│   └── prompts/             # External prompt files
├── tests/                   # Test suite
│   ├── unit/
│   ├── integration/
│   └── fixtures/            # Test data
└── scripts/                 # CLI/automation
```

### Structure Checklist

| Component | Required | Check |
|-----------|----------|-------|
| Agent definitions separated | Yes | [ ] |
| Tools in dedicated module | Yes | [ ] |
| Prompts externalized | Recommended | [ ] |
| Configuration separated | Yes | [ ] |
| Tests present | Yes | [ ] |
| Clear separation of concerns | Yes | [ ] |

## Design Pattern Checklist

### 1. Tool Design

**Required Patterns:**
- [ ] Tools have clear input/output schemas
- [ ] Tool errors return structured error responses
- [ ] Tools are stateless (no side effects on agent state)
- [ ] Tool timeouts are configured
- [ ] Tools validate inputs before execution

**BAD:**
```python
def search(query):
    return requests.get(f"https://api.com?q={query}").json()
```

**GOOD:**
```python
class SearchTool(BaseTool):
    name = "search"
    description = "Search the web for information"

    class InputSchema(BaseModel):
        query: str = Field(..., min_length=1, max_length=500)

    def execute(self, query: str) -> ToolResult:
        try:
            response = self.client.search(query, timeout=10)
            return ToolResult(success=True, data=response)
        except Timeout:
            return ToolResult(success=False, error="Search timed out")
        except Exception as e:
            return ToolResult(success=False, error=str(e))
```

### 2. Agent Loop

**Required Patterns:**
- [ ] Clear think → act → observe cycle
- [ ] Maximum iteration limit
- [ ] Graceful termination conditions
- [ ] State preserved between iterations
- [ ] Interrupt/cancel capability

**GOOD:**
```python
class Agent:
    MAX_ITERATIONS = 10

    async def run(self, task: str) -> AgentResult:
        state = AgentState(task=task)

        for i in range(self.MAX_ITERATIONS):
            if self._should_stop(state):
                break

            # Think
            action = await self.plan(state)

            # Act
            result = await self.execute(action)

            # Observe
            state = self.update_state(state, result)

        return self.finalize(state)
```

### 3. Memory Management

**Required Patterns:**
- [ ] Conversation history with size limits
- [ ] Summarization for long conversations
- [ ] Clear memory lifecycle (create, read, update, delete)
- [ ] Persistent storage for long-term memory
- [ ] Vector store for semantic retrieval (if RAG)

**Memory Types:**

| Type | Purpose | Persistence |
|------|---------|-------------|
| Working | Current task context | Session |
| Short-term | Recent conversation | Session |
| Long-term | User preferences, facts | Persistent |
| Episodic | Past task summaries | Persistent |
| Semantic | Embeddings/RAG | Persistent |

### 4. Error Handling

**Required Patterns:**
- [ ] Structured error types (not generic exceptions)
- [ ] Retry with exponential backoff for transient errors
- [ ] Graceful degradation (fallback behaviors)
- [ ] Error context preserved for debugging
- [ ] User-friendly error messages

**Error Categories:**

| Category | Retry | Action |
|----------|-------|--------|
| Rate limit | Yes | Exponential backoff |
| Timeout | Yes | Retry with longer timeout |
| Auth failure | No | Fail with clear message |
| Invalid input | No | Return validation error |
| Tool failure | Maybe | Try alternative tool |
| Model error | Yes | Retry or fallback model |

**GOOD:**
```python
class AgentError(Exception):
    def __init__(self, message: str, code: str, recoverable: bool = False):
        self.message = message
        self.code = code
        self.recoverable = recoverable

@retry(
    retry=retry_if_exception_type(RateLimitError),
    wait=wait_exponential(multiplier=1, max=60),
    stop=stop_after_attempt(3)
)
async def call_model(self, messages: list) -> str:
    try:
        return await self.client.complete(messages)
    except RateLimitError:
        raise  # Let retry handle it
    except AuthError as e:
        raise AgentError("Authentication failed", "AUTH_ERROR", recoverable=False)
```

### 5. State Management

**Required Patterns:**
- [ ] Immutable state updates (new state object per update)
- [ ] State schema validation
- [ ] State serialization for persistence
- [ ] Clear state transitions
- [ ] State versioning for migrations

**GOOD:**
```python
@dataclass(frozen=True)
class AgentState:
    task: str
    messages: tuple[Message, ...]
    tool_results: tuple[ToolResult, ...]
    iteration: int = 0
    status: Literal["running", "completed", "failed"] = "running"

    def with_message(self, message: Message) -> "AgentState":
        return replace(self, messages=self.messages + (message,))

    def with_tool_result(self, result: ToolResult) -> "AgentState":
        return replace(self, tool_results=self.tool_results + (result,))
```

### 6. Multi-Agent Coordination

**Patterns (if applicable):**
- [ ] Clear agent roles and responsibilities
- [ ] Message passing protocol defined
- [ ] Conflict resolution strategy
- [ ] Supervisor/orchestrator pattern
- [ ] Shared state management

**Coordination Patterns:**

| Pattern | Use Case |
|---------|----------|
| **Supervisor** | One agent routes to specialists |
| **Pipeline** | Sequential agent processing |
| **Debate** | Multiple agents propose, one decides |
| **Swarm** | Autonomous agents, shared goals |
| **Hierarchical** | Manager → workers structure |

### 7. Prompt Management

**Required Patterns:**
- [ ] System prompts externalized (not hardcoded)
- [ ] Prompt versioning
- [ ] Variables/templating for dynamic content
- [ ] Prompt testing/validation
- [ ] Clear prompt documentation

**GOOD:**
```python
# prompts/system.yaml
agent_system_prompt:
  version: "1.2"
  template: |
    You are a helpful assistant with access to these tools:
    {% for tool in tools %}