rai-architecture-review

Architecture Review

Purpose

Evaluate whether code is necessary and proportional using Beck's four rules of simple design. Core question: "Could we achieve the same outcome with less?"

Mastery Levels (ShuHaRi)

• Shu: Apply all heuristics systematically, explain each finding
• Ha: Focus on highest-signal heuristics for the scope, skip low-risk areas
• Ri: Pattern-match to known anti-patterns, minimal ceremony

Context

Inputs: Scope (story, epic, or bugfix), design doc, changed files from git diff (story/epic) or analysis.md + files at change points (bugfix).

Steps

Step 1: Load Design Context & Patterns

Before reviewing code, load what the design intended and what the codebase has learned:

1. Code orientation: Load SA-ranked code symbols for the current branch:

   rai session context -s code_context -p .
   

   Returns ~20 symbols ranked by structural proximity to active work modules. Empty result is valid. Use these as starting points — not exhaustive scope.

2. Read the design doc (design.md or scope.md) — what was the intended approach?

3. Query the knowledge graph for patterns in affected modules:

   Use raise_graph_query MCP tool with cwd="{project_or_worktree_path}", query="patterns for {affected_modules}". If MCP tools are not available, fall back to: rai graph query "patterns for {affected_modules}" --types pattern

   Use raise_pattern_query MCP tool with keywords="{module_keywords}", cwd="{project_or_worktree_path}". If MCP tools are not available, fall back to: rai graph query "{module_keywords}" --types pattern

4. Load established patterns (PAT-E-*) — these are proven solutions. Deviations need justification.

JIT: For deeper code exploration beyond the orientation map, query the graph directly:

rai graph query "symbol_name" --types symbol --limit 10
rai graph query "module_name" --module mod-raise-cli--session
rai graph query "callers of function_name" --types symbol

Use --file path/to/file.py to scope results to a specific file.

This context informs every subsequent step: you can't judge proportionality without knowing intent, and you can't catch regressions without knowing established patterns.

Step 2: Identify Scope and Changed Files

Detect the project language and filter by appropriate extensions:

1. Check .raise/manifest.yaml for project.language or project.project_type
2. Fallback: Scan extensions of changed files and pick the dominant language

# Story scope: files changed vs parent branch
git diff --name-only $(git merge-base HEAD <parent-branch>)..HEAD -- '<extensions>'
# Epic scope: all files changed vs development branch
git diff --name-only $(git merge-base HEAD <dev-branch>)..HEAD -- '<extensions>'
# Bugfix scope: no git diff — read analysis.md to identify the files at change points
# then read those files directly

Replace <extensions> with language-appropriate patterns (e.g., '*.py' '*.pyi' for Python, '*.ts' '*.tsx' for TypeScript, '*.cs' for C#).

Bugfix scope: No code has been written yet. Load work/bugs/{issue_key}/analysis.md, extract the files identified as change points, and read them. The question is: "Is the proposed fix approach proportional to the problem?"

Read every changed file and the design doc. You cannot judge proportionality without intent context.

Step 3: Necessity Audit (YAGNI — Beck Rule 4)

When a heuristic triggers, check: "Does the design doc justify this?" If yes, note as Observation.

Step 4: Proportionality Audit (KISS — Beck Rules 2+4)

Step 5: Duplication & Responsibility (Beck Rules 2-3)

Step 6: Agent-Authored Drift Audit

# Re-read manifest vars (PAT-129)
_CFG=$(rai manifest env)
DRIFT_CATALOG=$(echo "$_CFG" | python3 -c "import json,sys; print(json.load(sys.stdin).get('governance',{}).get('drift_catalog',''))")
DRIFT_HOTSPOTS=$(echo "$_CFG" | python3 -c "import json,sys; print(json.load(sys.stdin).get('governance',{}).get('drift_hotspots',''))")

# tier-3: print top-10 hotspot module ranks for cross-reference with changed modules (skipped if key absent)
if [ -n "$DRIFT_HOTSPOTS" ]; then
  python3 -c "
import json
hs = json.load(open('$DRIFT_HOTSPOTS'))
for e in hs.get('ranked_modules', [])[:10]:
    print(f\"  rank={e['rank']} id={e['id']} signals={e.get('signal_count','?')}\")
" 2>/dev/null || echo '⚠ Could not read hotspots file'
fi

Check for agent-specific drift patterns at both story and epic scope. If $DRIFT_CATALOG is declared in manifest, reference it §1 for full definitions — do not copy inline. If absent, use the inline AG1–AG6 table below as the sole reference (tier-3: silent skip). If $DRIFT_HOTSPOTS output was printed above, note which changed modules (if any) match top-ranked entries.

"No agent-authored drift detected" is always valid — do not invent findings.

Migration integrity check — runs once for every Alembic configuration whose configured script_location/versions/ contains a changed migration. Preserve the changed-path → configuration association: hybrid repositories may require more than one check.

BASE=$(git merge-base HEAD <parent-branch>)
mapfile -d '' CHANGED_FILES < <(git diff --name-only -z "$BASE"..HEAD)
declare -A VERIFIED_ROOTS=()

while IFS= read -r -d '' CONFIG; do
  CONFIG=${CONFIG#./}
  SCRIPT_ROOT=$(
    python3 - "$CONFIG" "$PWD" <<'PY'
from configparser import ConfigParser
from pathlib import Path
import sys

config = Path(sys.argv[1]).resolve()
repo = Path(sys.argv[2]).resolve()
parser = ConfigParser(defaults={"here": str(config.parent)})
parser.read(config, encoding="utf-8")
location = parser.get("alembic", "script_location", fallback="").strip()
if location and ":" not in location:
    root = Path(location)
    root = root if root.is_absolute() else config.parent / root
    try:
        print(root.resolve().relative_to(repo))
    except ValueError:
        pass
PY
  )
  [ -n "$SCRIPT_ROOT" ] || continue

  for CHANGED in "${CHANGED_FILES[@]}"; do
    case "$CHANGED" in
      "$SCRIPT_ROOT"/versions/*.py)
        CONFIG_DIR=$(dirname "$CONFIG")
        CONFIG_NAME=$(basename "$CONFIG")
        (cd "$CONFIG_DIR" && alembic -c "$CONFIG_NAME" heads)
        VERIFIED_ROOTS["$SCRIPT_ROOT"]=1
        break
        ;;
    esac
  done
done < <(find . -type f -name alembic.ini \
  -not -path './.git/*' -not -path './.venv/*' -print0)

# Any changed versions file not associated with a discovered configuration is
# an explicit unverified outcome, never a silent success.
for CHANGED in "${CHANGED_FILES[@]}"; do
  case "$CHANGED" in
    versions/*.py|*/versions/*.py)
      VERIFIED=0
      for SCRIPT_ROOT in "${!VERIFIED_ROOTS[@]}"; do
        case "$CHANGED" in
          "$SCRIPT_ROOT"/versions/*.py) VERIFIED=1; break ;;
        esac
      done
      if [ "$VERIFIED" -eq 0 ]; then
        echo "⚠ Migration '$CHANGED' changed but no applicable Alembic configuration was found; migration integrity cannot be verified."
      fi
      ;;
  esac
done

Step 7: Portfolio Impact Check

Using the modules already identified in Step 6 drift analysis, cross-reference against the portfolio cartridge to detect concurrent initiatives that may conflict.

For each touched module, run:

rai graph context mod-{module} --format json 2>/dev/null || true

Extract the portfolio_impact field from the JSON output. Handle each case:

Fail-open rule: A missing or unavailable portfolio cartridge must never block the AR. This check is informative, not a gate.

Step 8: Systemic Audit (Epic Scope Only)

Skip for story scope. Cross-module heuristics:

Step 9: Lean Compliance Audit

Verify the implementation follows the lean principles established in design:

For each L-finding: cite the pattern or design decision that was violated.

Step 10: Present Findings

## Architecture Review: {id} (scope: {story|epic})

### Critical (fix before merge)
### Recommended (simplify before next cycle)
### Questions (require human judgment)
### Observations (patterns noted)
### Verdict
- [ ] PASS / PASS WITH QUESTIONS / SIMPLIFY

Every finding: specific file:line, heuristic ID, proportionality concern, concrete simplification.

Step 11: Write Review Artifact

The bugfix and story pipelines validate this exact file before the phase can advance — writing it is not optional in those scopes (RAISE-16030).

Use raise_docs_write MCP tool with doc_type="architecture-review", title="{id}: architecture review", content="[Step 10 findings, verbatim]", output_path="{path from table above}", cwd="{project_or_worktree_path}". If MCP tools are not available, fall back to:

rai docs write architecture-review \
  --title "{id}: architecture review" \
  --stdin \
  --output-path {path from table above} << 'EOF'
[Step 10 findings, verbatim]
EOF

git add {path from table above}

Output

Quality Checklist

• Design doc and graph patterns loaded before reviewing (Step 1)
• Project language detected before filtering files
• All changed files for detected language read before reviewing
• Every finding cites specific file:line and heuristic ID (AG1-AG6, H1-H16, L1-L5)
• Portfolio Impact Check run; breaking conflicts flagged HIGH or cartridge advisory emitted (Step 7)
• Lean compliance verified: MVP, pattern adherence, no gold-plating
• Known patterns (PAT-E-*) checked against implementation
• Questions ratio >30% of findings (humility signal)
• "No issues found" is a valid outcome — do not invent findings

References

• Evidence: work/research/architecture-review/
• Complements: /rai-quality-review (correctness), /rai-story-review (retrospective)
• Framework: Beck's Simple Design Rules, Fowler's Code Smells, Silva et al. (ESEM 2024)