Architecture Review

Review the system behind the diff.

See what a change does to the system.

A new cache can introduce a reversed dependency, an import cycle, and a deployment requirement. Keep the evidence together in one architectural review.

Findings · 5

Architecture · PR #412 · Cache generated reports in Redis

Layers touched delivery · orchestration · adaptersShips in code-review-api · code-review-burst-worker

What the change did to the layers

Delivery sits above orchestration and adapters. The existing trigger_review call goes from api-main to review-pipeline. A new upward import from review-pipeline to api-main closes an import cycle. New calls from api-main and review-worker reach report-cache, which introduces a Redis boundary outside the process. Redis is not provisioned in either deployable.

deliveryentry points
orchestrationcoordinates work
adaptersexternal systems
outside the process
api-main
routes.py
review-pipeline
main_v2.py
review-worker
review_worker.py
report-cachenew
report_cache.py
Redis
trigger_reviewexisting
UPWARD · 0.74store_report_in_mongocloses an import cycle
ReportCache.get
ReportCache.put
new boundary: cache
first cache in the repository not provisioned in either deployable
  • Added call
  • Violation
  • New boundary
  • Existing call completing a cycle

Findings

  1. High

    Orchestration → delivery: main_v2.py imports routes.py for store_report_in_mongo

    Every other orchestration module sits below the routes. The call goes up a rank.

    0.74 · inferred
  2. High

    Import cycle: routes.py ↔ main_v2.py

    routes.py already calls trigger_review; this change closes the loop.

    1.00 · proof
  3. Medium

    New boundary: report_cache.py is the first module in the repository to call Redis

    No layer touched a cache before. Existing adapters carry http · db · vector · queue · llm.

    Measured
  4. Medium

    REDIS_URL is read but set in no deployable's environment

    Neither code-review-api nor code-review-burst-worker names it in deployment.yaml.

    Measured
  5. Low

    report_cache.py logs via logging.getLogger

    139 of 141 modules use create_logger. Confirmed as a deviation, not a deliberate exception.

    Confirmed

Worth a senior look

Failure modeBoundary on a request path
GET .../report now calls Redis before Mongo. No timeout is set on the client, and an unreachable Redis raises rather than falling through to Mongo. The route returns a 500 instead of getting slower.
OperationalNew dependency, new environment
Redis is now a dependency of two deployables and provisioned in neither. The burst worker also needs REDIS_URL; today it has only the queue and Mongo.
ConsistencyWrite and read in different units
handle_feedback.py updates generated_reports directly and nothing invalidates the cached copy. A corrected report is served stale until TTL.
Pattern check report-cache has no siblings: nothing to comparePre-existing violations in touched files 0Model current · context_rev a1b2c3dModel calls 2: one confirm, one explanation
The same report as a terminal or Jira comment
Architecture - PR #412  Cache generated reports in Redis
Verdict: FINDINGS (5)    ships in code-review-api, code-review-burst-worker

delivery       api-main / routes.py
                 get_report ----> adapters/report-cache . ReportCache.get
                 trigger_review -> orchestration/review-pipeline . main_v2  (existing)

orchestration  review-worker / review_worker.py
                 store ----> adapters/report-cache . ReportCache.put
               review-pipeline / main_v2.py
                 import --!-> delivery/api-main . store_report_in_mongo  UPWARD [0.74]
                              closes a cycle with trigger_review

adapters       report-cache / report_cache.py   (+ new)
                 ReportCache ----> [redis]   boundary: cache - first in the repository,
                                            provisioned in no deployable

+ added   ! violation   [ ] boundary   (existing) drawn only where it completes a cycle

Findings
  1. [high]   orchestration -> delivery: main_v2.py imports routes.py for store_report_in_mongo [0.74]
  2. [high]   import cycle: routes.py <-> main_v2.py                                         [1.00]
  3. [medium] new boundary: report_cache.py is the first module in the repository to call Redis
  4. [medium] REDIS_URL is read but set in no deployable's environment
  5. [low]    report_cache.py logs via logging.getLogger; 139 of 141 modules use create_logger

An unusual dependency is not automatically wrong. Intentional exceptions should have an explicit rationale and the right owner's review.

Four ways to review
the system behind the diff.

Line-by-line review sees what changed. Architecture Review shows where the change sits in the system, and which design decisions need an owner's attention.

Detect architectural violations and dependency issues.

Identify violations of architectural layers, improper dependencies, circular imports, and skipped layers. Ensure code changes adhere to established architectural boundaries and design principles.

  • Layer boundariesViolations & skipped layers
  • ImportsCircular dependencies
  • Dependency directionImproper dependencies
See the findings

Assess the architectural impact of code changes.

Understand how PRs affect system components, services, infrastructure, data stores, and external dependencies. Visualize architectural changes before they introduce unintended complexity or risks.

  • Components & servicesWhat the PR touches
  • Infrastructure & data storesNew and changed boundaries
  • External dependenciesCalls leaving the system
See the layer diagram

Identify cross-repository and integration risks.

Detect changes that impact dependent modules, shared interfaces, or connected repositories. Identify potential breaking changes and unexpected dependencies before they reach production.

  • Dependent modulesCallers of the change
  • Shared interfacesContract changes
  • Connected repositoriesPotential breaking changes
Run it on a pull request

Surface design decisions requiring senior engineering review.

Highlight architectural concerns involving new databases, caches, queues, external services, failure handling, and deployment configuration. Provide evidence-based findings and questions to guide senior engineers in validating critical design decisions.

  • New infrastructureDatabases, caches & queues
  • Failure handlingTimeouts & fallbacks
  • DeploymentConfiguration & environment
See what's worth a senior look

Compare the change with the system it joins.

Compare established patterns.

Check the PR against the design principles and conventions used across the repository and organization.

Identify layer violations.

Surface dependencies that cross abstraction boundaries or conflict with the established architecture.

Explain changes by layer.

Group related modifications into semantic layers and explain how the pieces of the change work together.

Commands

Claude Code
cloudaeye:review-arch <pr#>
GitHub · Bitbucket · GitLab
@cloudaeye /review_arch

Local-diff support is under consideration, not a confirmed capability.