Agent-almanac review-software-architecture

install
source · Clone the upstream repo
git clone https://github.com/pjt222/agent-almanac
Claude Code · Install into ~/.claude/skills/
T=$(mktemp -d) && git clone --depth=1 https://github.com/pjt222/agent-almanac "$T" && mkdir -p ~/.claude/skills && cp -r "$T/i18n/wenyan/skills/review-software-architecture" ~/.claude/skills/pjt222-agent-almanac-review-software-architecture-350a7c && rm -rf "$T"
manifest: i18n/wenyan/skills/review-software-architecture/SKILL.md
source content

Review 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 LevelDescriptionExample
Low (good)Modules communicate through interfacesService A calls Service B's API
MediumModules share data structuresShared DTO/model library
High (concern)Modules reference each other's internalsDirect database access across modules
PathologicalModules modify each other's internal stateGlobal 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

PrincipleQuestionRed Flags
Single ResponsibilityDoes each class/module have one reason to change?Classes with >5 public methods on unrelated concerns
Open/ClosedCan behavior be extended without modifying existing code?Frequent modifications to core classes for each new feature
Liskov SubstitutionCan subtypes replace their base types without breaking behavior?Type checks (
instanceof
) scattered through consumer code
Interface SegregationAre interfaces focused and minimal?"Fat" interfaces where consumers implement unused methods
Dependency InversionDo 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
    code-reviewer
    for PR-level feedback.
  • 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-audit-codebase
    — security-focused code and configuration review
  • configure-git-repository
    — repository structure and conventions
  • design-serialization-schema
    — data schema design and evolution
  • review-data-analysis
    — review of analytical correctness (complementary perspective)