Agent-almanac review-software-architecture
git clone https://github.com/pjt222/agent-almanac
T=$(mktemp -d) && git clone --depth=1 https://github.com/pjt222/agent-almanac "$T" && mkdir -p ~/.claude/skills && cp -r "$T/i18n/caveman-lite/skills/review-software-architecture" ~/.claude/skills/pjt222-agent-almanac-review-software-architecture && rm -rf "$T"
i18n/caveman-lite/skills/review-software-architecture/SKILL.mdReview Software Architecture
Evaluate software architecture at the system level for quality attributes, design principles adherence, and long-term maintainability.
When to Use
- Evaluating a proposed architecture before implementation begins
- Assessing an existing system for scalability, maintainability, or security
- Reviewing Architecture Decision Records (ADRs) for a project
- Performing a technical debt assessment
- Evaluating whether a system is ready for a significant scale-up or feature expansion
- Differentiating from line-level code review (which focuses on PR-level changes)
Inputs
- Required: System codebase or architecture documentation (diagrams, ADRs, README)
- Required: Context about the system's purpose, scale, and constraints
- Optional: Non-functional requirements (latency, throughput, availability targets)
- Optional: Team size and skill composition
- Optional: Technology constraints or preferences
- Optional: Known pain points or areas of concern
Procedure
Step 1: Understand the System Context
Map the system boundaries and interfaces:
## System Context - **Name**: [System name] - **Purpose**: [One-line description] - **Users**: [Who uses it and how] - **Scale**: [Requests/sec, data volume, user count] - **Age**: [Years in production, major versions] - **Team**: [Size, composition] ## External Dependencies | Dependency | Type | Criticality | Notes | |-----------|------|-------------|-------| | PostgreSQL | Database | Critical | Primary data store | | Redis | Cache | High | Session store + caching | | Stripe | External API | Critical | Payment processing | | S3 | Object storage | High | File uploads |
Expected: Clear picture of what the system does and what it depends on. On failure: If architecture documentation is missing, derive the context from code structure, configs, and deployment files.
Step 2: Evaluate Structural Quality
Coupling Assessment
Examine how tightly modules depend on each other:
- Dependency direction: Do dependencies flow in one direction (layered) or circular?
- Interface boundaries: Are modules connected through defined interfaces/contracts or direct implementation references?
- Shared state: Is mutable state shared between modules?
- Database coupling: Do multiple services read/write the same tables directly?
- Temporal coupling: Must operations happen in a specific order without explicit orchestration?
# Detect circular dependencies (JavaScript/TypeScript) npx madge --circular src/ # Detect import patterns (Python) # Look for deep cross-package imports grep -r "from app\." --include="*.py" | sort | uniq -c | sort -rn | head -20
Cohesion Assessment
Evaluate whether each module has a single, clear responsibility:
- Module naming: Does the name accurately describe what the module does?
- File size: Are files or classes excessively large (>500 lines suggests multiple responsibilities)?
- Change frequency: Do unrelated features require changes to the same module?
- God objects: Are there classes/modules that everything depends on?
| Coupling Level | Description | Example |
|---|---|---|
| Low (good) | Modules communicate through interfaces | Service A calls Service B's API |
| Medium | Modules share data structures | Shared DTO/model library |
| High (concern) | Modules reference each other's internals | Direct database access across modules |
| Pathological | Modules modify each other's internal state | Global mutable state |
Expected: Coupling and cohesion assessed with specific examples from the codebase. On failure: If the codebase is too large for manual review, sample 3-5 key modules and the most-changed files.
Step 3: Assess SOLID Principles
| Principle | Question | Red Flags |
|---|---|---|
| Single Responsibility | Does each class/module have one reason to change? | Classes with >5 public methods on unrelated concerns |
| Open/Closed | Can behavior be extended without modifying existing code? | Frequent modifications to core classes for each new feature |
| Liskov Substitution | Can subtypes replace their base types without breaking behavior? | Type checks () scattered through consumer code |
| Interface Segregation | Are interfaces focused and minimal? | "Fat" interfaces where consumers implement unused methods |
| Dependency Inversion | Do high-level modules depend on abstractions, not details? | Direct instantiation of infrastructure classes in business logic |
## SOLID Assessment | Principle | Status | Evidence | Impact | |-----------|--------|----------|--------| | SRP | Concern | UserService handles auth, profile, notifications, and billing | High — changes to billing risk breaking auth | | OCP | Good | Plugin system for payment providers | Low | | LSP | Good | No type-checking anti-patterns found | Low | | ISP | Concern | IRepository has 15 methods, most implementors use 3-4 | Medium | | DIP | Concern | Controllers directly instantiate database repositories | Medium |
Expected: Each principle assessed with at least one specific example. On failure: Not all principles apply equally to every architecture style. Note when a principle is less relevant (e.g., ISP matters less in functional codebases).
Step 4: Review API Design
For systems that expose APIs (REST, GraphQL, gRPC):
- Consistency: Naming conventions, error formats, pagination patterns uniform
- Versioning: Strategy exists and is applied (URL, header, content negotiation)
- Error handling: Error responses are structured, consistent, and don't leak internals
- Authentication/Authorization: Properly enforced at the API layer
- Rate limiting: Protection against abuse
- Documentation: OpenAPI/Swagger, GraphQL schema, or protobuf definitions maintained
- Idempotency: Mutating operations (POST/PUT) handle retries safely
## API Design Review | Aspect | Status | Notes | |--------|--------|-------| | Naming consistency | Good | RESTful resource naming throughout | | Versioning | Concern | No versioning strategy — breaking changes affect all clients | | Error format | Good | RFC 7807 Problem Details used consistently | | Auth | Good | JWT with role-based scopes | | Rate limiting | Missing | No rate limiting on any endpoint | | Documentation | Concern | OpenAPI spec exists but 6 months out of date |
Expected: API design reviewed against common standards with specific findings. On failure: If no API is exposed, skip this step and focus on internal module interfaces.
Step 5: Evaluate Scalability and Reliability
- Statelessness: Can the application scale horizontally (no local state)?
- Database scalability: Are queries indexed? Is the schema suitable for the data volume?
- Caching strategy: Is caching applied at appropriate layers (database, application, CDN)?
- Failure handling: What happens when a dependency is unavailable (circuit breaker, retry, fallback)?
- Observability: Are logs, metrics, and traces implemented?
- Data consistency: Is eventual consistency acceptable or is strong consistency required?
Expected: Scalability and reliability assessed relative to stated non-functional requirements. On failure: If non-functional requirements are undocumented, recommend defining them as a first step.
Step 6: Assess Technical Debt
## Technical Debt Inventory | Item | Severity | Impact | Estimated Effort | Recommendation | |------|----------|--------|-----------------|----------------| | No database migrations | High | Schema changes are manual and error-prone | 1 sprint | Adopt Alembic/Flyway | | Monolithic test suite | Medium | Tests take 45 min, developers skip them | 2 sprints | Split into unit/integration/e2e | | Hardcoded config values | Medium | Environment-specific values in source code | 1 sprint | Extract to env vars/config service | | No CI/CD pipeline | High | Manual deployment prone to errors | 1 sprint | Set up GitHub Actions |
Expected: Technical debt catalogued with severity, impact, and effort estimates. On failure: If the debt inventory is overwhelming, prioritize the top 5 items by impact/effort ratio.
Step 7: Review Architecture Decision Records (ADRs)
If ADRs exist, evaluate:
- Decisions have clear context (what problem was being solved)
- Alternatives were considered and documented
- Trade-offs are explicit
- Decisions are still current (not superseded without documentation)
- New significant decisions have ADRs
If ADRs don't exist, recommend establishing them for key decisions.
Step 8: Write the Architecture Review
## Architecture Review Report ### Executive Summary [2-3 sentences: overall health, key concerns, recommended actions] ### Strengths 1. [Specific architectural strength with evidence] 2. ... ### Concerns (by severity) #### Critical 1. **[Title]**: [Description, impact, recommendation] #### Major 1. **[Title]**: [Description, impact, recommendation] #### Minor 1. **[Title]**: [Description, recommendation] ### Technical Debt Summary [Top 5 debt items with prioritized recommendations] ### Recommended Next Steps 1. [Actionable recommendation with clear scope] 2. ...
Expected: Review report is actionable with prioritized recommendations. On failure: If the review is time-boxed, clearly state what was covered and what remains unassessed.
Validation
- System context documented (purpose, scale, dependencies, team)
- Coupling and cohesion assessed with specific code examples
- SOLID principles evaluated where applicable
- API design reviewed (if applicable)
- Scalability and reliability assessed against requirements
- Technical debt catalogued and prioritized
- ADRs reviewed or their absence noted
- Recommendations are specific, prioritized, and actionable
Common Pitfalls
- Reviewing code instead of architecture: This skill is about system-level design, not line-level code quality. Use
for PR-level feedback.code-reviewer - Prescribing a specific technology: Architecture reviews should identify problems, not mandate specific tools unless there's a clear technical reason.
- Ignoring team context: The "best" architecture for a 3-person team differs from a 30-person team. Consider organizational constraints.
- Perfectionism: Every system has tech debt. Focus on debt that is actively causing pain or blocking future work.
- Assuming scale: Don't recommend distributed systems for an app serving 100 users. Match architecture to actual requirements.
Related Skills
— security-focused code and configuration reviewsecurity-audit-codebase
— repository structure and conventionsconfigure-git-repository
— data schema design and evolutiondesign-serialization-schema
— review of analytical correctness (complementary perspective)review-data-analysis