Purpose: Standardized evaluation framework for reviewing architectural changes (ADRs, specs, governance config changes) Audience: Project owner, future contributors, AI assistants preparing proposals Version: 1.0 Date: 2025-12-28
Without a rubric, architectural reviews are:
- Subjective: "I don't like this" without specific criteria
- Inconsistent: Different standards applied to similar changes
- Incomplete: Miss critical aspects (security, observability, etc.)
- Inefficient: Repeated back-and-forth on same issues
A structured rubric provides:
- ✅ Clear criteria: Explicit standards for acceptance
- ✅ Consistency: Same lens applied to all changes
- ✅ Completeness: Ensures all concerns addressed
- ✅ Efficiency: AI assistants can self-check before submission
- ✅ Learning: Junior contributors understand expectations
Apply this rubric to:
- ADRs (Architecture Decision Records)
- Component specifications (new or major revisions)
- Governance config changes (mode thresholds, tool permissions)
- Captain's Log proposals (agent-generated improvements)
- Experimental changes (hypothesis-driven modifications)
Not for: Minor doc fixes, session logs, routine bug fixes
Each review category has:
- Questions to guide evaluation
- Scoring (Pass/Fail or 0-5 scale)
- Severity (Blocking, High Priority, Nice-to-Have)
- Is the problem/need clearly stated? (e.g., "Orchestrator needs error handling strategy")
- Is the impact explained? (e.g., "Without this, crashes cause data loss")
- Are specific examples provided? (not just abstract descriptions)
- Is urgency/priority justified? (why now vs later?)
- Pass: Problem statement is clear, impactful, concrete
- Fail: Vague, unclear why this matters, or no real problem identified
Severity: 🚨 Blocking (can't evaluate solution without clear problem)
- Does the solution address the stated problem? (not a tangent)
- Is it technically sound? (no obvious flaws)
- Is it implementable within project constraints? (available models, tools, time)
- Does it align with architectural principles? (safety-first, observability, determinism, etc.)
- Are interfaces and contracts well-defined? (clear APIs, data formats)
- 5: Excellent—elegant, simple, comprehensive
- 4: Good—sound with minor refinements needed
- 3: Acceptable—works but not optimal
- 2: Weak—significant concerns
- 1: Poor—major flaws
- 0: Unacceptable—fundamentally broken
Severity: 🚨 Blocking if score < 3
- Are at least 2 alternatives listed? (beyond chosen solution)
- Are alternatives realistic? (not strawmen)
- Are tradeoffs explicit? (pros/cons of each option)
- Is the choice justified? (why this option over others?)
- Pass: Multiple real alternatives, clear tradeoffs, justified choice
- Fail: No alternatives OR only trivial/strawman options
Severity: 🔴 High Priority for ADRs (blocking),
- Are positive consequences listed? (what improves)
- Are negative consequences acknowledged? (costs, risks, complexity)
- Are downstream impacts identified? (what else changes as a result)
- Is migration cost assessed? (if changing existing system)
- Are rollback/exit strategies discussed? (if experiment fails)
- Pass: Balanced view of positive/negative, downstream impacts considered
- Fail: Only upsides listed OR critical impacts missed
Severity: 🔴 High Priority (can accept with conditions, but must address)
- How will this be monitored? (metrics, logs, traces)
- How will failures be detected? (alerts, error signals)
- How will issues be debugged? (diagnostic tools, log queries)
- Are telemetry events defined? (what gets logged when)
- Pass: Clear observability story, integrated with existing telemetry
- Fail: No mention of monitoring OR tacked-on afterthought
Severity: 🚨 Blocking for system components,
- Are security implications assessed? (data exposure, privilege escalation, etc.)
- Are governance constraints integrated? (mode checks, permissions, approvals)
- Are error/failure modes handled safely? (fail-safe vs fail-open)
- Are secrets/PII protected? (no hard-coded secrets, redaction in logs)
- Pass: Security reviewed, governance integrated, safe failure modes
- Fail: Security not considered OR unsafe behavior possible
Severity: 🚨 Blocking if any safety risk identified
- How will this be tested? (unit, integration, manual)
- Are test cases listed? (happy path + failure scenarios)
- Is validation automated or manual? (prefer automated)
- Are acceptance criteria defined? (how to know it works)
- Pass: Clear test strategy, acceptance criteria defined
- Fail: No test plan OR "we'll test it later"
Severity: 🔴 High Priority for core components,
- Is the change documented? (ADR, spec update, or Captain's Log)
- Are related docs updated? (no orphaned references)
- Is terminology consistent? (uses project vocabulary)
- Are diagrams provided? (if complex relationships)
- Is the writing clear? (not overly verbose or jargon-heavy)
- Pass: Well-documented, clear, consistent
- Fail: Poor documentation OR inconsistent with existing docs
Severity:
- Is the scope reasonable? (not too ambitious for one change)
- Can it be broken into smaller pieces? (prefer incremental)
- Is the complexity justified? (not over-engineered)
- Are future extensions considered? (room to grow)
- Pass: Right-sized scope, manageable complexity
- Fail: Scope creep OR unnecessarily complex
Severity: 🔴 High Priority (can request scope reduction)
- Aligns with biological metaphor? (if applicable)
- Maintains homeostasis focus? (control loops, stability)
- Preserves determinism where needed? (orchestrator, governance)
- Supports transparency? (observable behavior)
- Respects human-first control? (no silent autonomy)
- Local-first? (no unnecessary cloud dependencies)
- Pass: Consistent with project philosophy
- Fail: Violates core principles
Severity: 🚨 Blocking if principle violation is fundamental
| Category | Weight | Score | Weighted Score |
|---|---|---|---|
| 1. Problem Clarity | 10% | Pass/Fail | — |
| 2. Solution Quality | 25% | 0-5 | × 0.25 |
| 3. Alternatives | 10% | Pass/Fail | — |
| 4. Consequences | 10% | Pass/Fail | — |
| 5. Observability | 15% | Pass/Fail | — |
| 6. Security & Safety | 15% | Pass/Fail | — |
| 7. Testing Strategy | 10% | Pass/Fail | — |
| 8. Documentation | 5% | Pass/Fail | — |
| 9. Scope & Complexity | — | Qualitative | — |
| 10. Consistency | — | Pass/Fail | Blocking if Fail |
- Auto-Approve: All blocking items Pass, Solution Quality ≥ 4
- Approve with Conditions: 1-2 High Priority items need fixes
- Request Revisions: Multiple High Priority failures OR Solution Quality < 3
- Reject: Any Blocking item fails OR fundamental principle violation
Before proposing an architectural change:
- Self-score using this rubric
- Fix obvious gaps (missing alternatives, no observability, etc.)
- Highlight uncertainties (can't validate security? Say so)
- Include rubric score in proposal (e.g., "Self-assessed: 4/5 Solution Quality")
When reviewing a proposal:
- Skim for blocking issues first (security, principle violations)
- Score each category systematically
- Document decision with rubric scores (not just "looks good")
- Provide specific feedback (reference rubric categories)
- Request revisions or approve with clear rationale
- AI assistants catch issues early (self-check before submission)
- Fewer review cycles (comprehensive first pass)
- Clear criteria reduce debate
- Nothing slips through (checklist ensures completeness)
- Consistent standards across all changes
- Learning reinforcement (AI assistants improve over time)
- Specific feedback ("Observability section missing" vs "needs work")
- Shared vocabulary (rubric categories)
- Justified decisions (scores, not opinions)
- Security always checked (not an afterthought)
- Safety implications explicit (governance integration verified)
- Testing non-negotiable (no "we'll test later")
Proposal: Switch orchestrator from sync to async execution
| Category | Score | Notes |
|---|---|---|
| Problem Clarity | ✅ Pass | Clear: "Need concurrent tool execution for performance" |
| Solution Quality | 4/5 | Sound, but migration cost high (see below) |
| Alternatives | ✅ Pass | Sync-first, threadpool, async—compared explicitly |
| Consequences | Positive clear, but migration testing not detailed | |
| Observability | ✅ Pass | Async spans defined, no change to telemetry model |
| Security & Safety | ✅ Pass | Governance hooks preserved, no new risks |
| Testing Strategy | Unit tests listed, but integration test plan vague | |
| Documentation | ✅ Pass | ADR complete, orchestrator spec updated |
| Scope & Complexity | 🔴 Flag | Large scope—could break into phases? |
| Consistency | ✅ Pass | Aligns with principles |
Conditions:
- Add detailed migration testing plan (Category 7)
- Address consequences—rollback strategy if async causes issues (Category 4)
- Consider phased rollout—async tools first, full orchestrator later (Category 9)
Rationale: Solution is sound (4/5), but execution risk requires more detail on testing and phasing.
This rubric will evolve based on:
- Recurring review issues (add category if pattern emerges)
- Project maturity (raise bar as system stabilizes)
- New risks (add security checklist items as threats identified)
Review rubric quarterly or after major project milestones.
- Problem unclear or non-existent
- Solution technically unsound (score < 3)
- Security risk identified
- No observability plan (for system components)
- Violates core principles
- No alternatives listed (ADRs)
- Consequences incomplete
- Testing strategy vague
- Scope too large
- Documentation needs polish
- Minor consistency issues
- Nice-to-have diagrams missing
| Stakeholder | Benefit |
|---|---|
| Project Owner | Faster, higher-quality reviews; clear acceptance criteria |
| AI Assistants | Self-improvement loop; learn what "good" looks like |
| Future Contributors | Transparent expectations; consistent standards |
| Project | Risk reduction; better architecture; maintainable docs |
This rubric ensures every architectural change is evaluated rigorously, consistently, and efficiently.
| Version | Date | Changes |
|---|---|---|
| 1.0 | 2025-12-28 | Initial PR review rubric created |