architecture-review

Files (1)

• SKILL.md

SKILL.md (first lines)

# Architecture Review

## Contents

- **Read scope** - how to consume this skill when a read is truncated.
- **Objective** - the question a review answers; cross-language scope.
- **Review priorities** - the 12-point precedence order; a lower number wins.
- **Establish the evidence** - what to inspect and when to ask.
- **Boundary and ownership checks** - real responsibilities, leaks,
  authoritative owners.
- **Cohesion, depth, and locality checks** - module purpose, fragmentation,
  change locality.
- **Dependency direction checks** - inward leaks, cycles, needless inversions.
- **SOLID checks** - SRP/OCP/LSP/ISP/DIP with architectural consequences only.
- **Semantic DRY checks** - harmful duplication versus harmful abstraction.
- **API and data-shape checks** - parameter grouping and value objects,
  including CancellationToken checks.
- **Clean Architecture checks** - boundary value, not diagram resemblance.
- **Review output** - finding classes (Blocking, Significant, Minor,
  Observation) and the five-part finding format.
- **Skill boundaries** - when to load design, code-review, domain-modeling, or
  tdd.

## Read scope

Read this entire skill, including every section listed in Contents, before
applying it. If a read is truncated, continue from the next unread line until
the end.

## Objective

Challenge whether the architecture is good, not whether it resembles a preferred
pattern. Do not redesign everything because another architecture is
theoretically cleaner. Work across C#, ASP.NET, C++, Rust, Go, and TypeScript,
respecting each repository's conventions.

## Review priorities

Review in this order; a lower number wins:

1. Requirements and intended behaviour.
2. Existing repository architecture and conventions.
3. Dependency direction.
4. Ownership and boundaries.
5. Cohesion and module depth.
6. Locality of change.
7. Abstraction quality.
8. API ergonomics.
9. SOLID concerns with architectural consequences.
10. DRY concerns or harmful abstractions.
11. Clean Architecture boundary concerns.
12. Unnecessary complexity.

## Establish the evidence

1. Read applicable `AGENTS.md` instructions, requirements, and architectural
   documentation.
2. Inspect the reviewed design or implementation, its callers, neighbouring
   modules, dependency direction, existing abstractions, state and behaviour
   owners, and tests at established seams.
3. Evaluate against the repository, not in isolation.
4. Investigate facts yourself. Ask only for missing review scope, intended
   behaviour, or user-owned constraints that change the verdict.
5. Distinguish demonstrated problems from unverified risks.
6. A proposed design can be reviewed without a diff; require a branch or
   merge-base only when the requested scope needs one.

Use the checks below as diagnostic guidance, not automatic violations. Ground
findings in demonstrated consequences and the repository's requirements.

## Boundary and ownership checks

- Does each boundary represent a real responsibility?
- Is a dependency crossing a meaningful architectural boundary?
- Is infrastructure leaking into domain or application code unnecessarily?
- Are framework concerns infecting core logic?
- Is the proposed boundary improving isolation?
- Is there one authoritative owner of each state and behaviour, with a clear
  lifetime?

Conceptually: domain owns business concepts, rules, invariants, and behaviour;
application owns use cases and orchestration; infrastructure supplies external
mechanisms; adapters handle transport and presentation. These are useful
concepts, not mandatory directories or a reason to impose competing layers.

## Cohesion, depth, and locality checks

- Does this module have a strong purpose?
- Are related decisions kept together?
- Is behaviour separated from the data or policy it naturally belongs with?
- Has the design fragmented into too many tiny abstractions?
- Would one likely requirement change force scattered edits across unrelated
  modules?
- Does each abstraction hide meaningful complexity, or merely rename an
  implementation?
- Are recommendations tied to a concrete change scenario rather than
  hypothetical flexibility?

## Dependency direction checks

- Infrastructure leaking inward.
- Domain depending directly on infrastructure.
- Application logic coupled to transport or framework concerns.
- Circular dependencies.
- Abstractions that exist only to reverse an otherwise harmless dependency.

Explain the practical harm in context. Do not require every dependency to be
inverted and do not treat every concrete dependency as a defect.

## SOLID checks

Report only violations with architectural consequences:

- **SRP**: multiple independent reasons to change, not class size.
- **OCP**: a real axis of variation obstructed by the design, not a missing
  speculative extension point.
- **LSP**: an implementation that breaks its abstraction's behavioural contract.
- **ISP**: consumers forced to depend on capabilities they do not need.
- **DIP**: a dependency direction that harms isolation or testability.

From mineugene/pi-dev-config, MIT.

More from mineugene/pi-dev-config