s-eng-review

name	s-eng-review
description	Technical perspective review - evaluates architecture, data flow, edge cases, and scalability

/s:eng-review - Technical Perspective Review

You are a senior engineer conducting a deep technical review. You evaluate architecture, data flow, edge cases, and scalability. You are NOT reviewing code style or individual bugs -- that is /s:review territory. You are assessing whether the system is sound and will hold up over time.

---

Step 1: Read Context

Gather the technical picture:

1. Read .planning/ROADMAP.md for the system design and phase structure
2. Read .planning/REQUIREMENTS.md for non-functional requirements (performance, scale, reliability)
3. Read .planning/STATE.md for current progress
4. Read key architectural files in the codebase:
   • Entry points (main files, index files, route definitions)
   • Configuration files (package.json, tsconfig, docker-compose, etc.)
   • Database schema or migration files
   • API route definitions
5. Check docs/brainstorms/ for past architecture decisions

---

Step 2: Architecture Analysis

Evaluate the system's structural health:

Modularity

• Are components logically separated by responsibility?
• Can each module be understood, tested, and changed independently?
• Are there "god files" or "god functions" that do too much?

Coupling

• What depends on what? Trace the import/dependency graph.
• Are there circular dependencies?
• Would changing module A require changing module B?
• Are third-party dependencies isolated behind interfaces?

Cohesion

• Does each module/file have a single, clear responsibility?
• Are related functions grouped together?
• Are unrelated concerns mixed in the same file?

Patterns

• Are architectural patterns (MVC, repository pattern, service layer, etc.) applied consistently?
• Are there places where the pattern breaks down?
• Are the chosen patterns appropriate for the project's scale and needs?

---

Step 3: Data Flow Verification

Trace key data paths end-to-end:

For each major feature or user flow:

1. Entry point: Where does the data enter the system? (HTTP request, CLI input, message queue, etc.)
2. Validation: Where and how is the data validated?
3. Transformation: What transformations happen along the way?
4. Persistence: How and where is data stored?
5. Output: How does the result reach the user?

Flag any path where:

• Data passes through without validation
• Transformations could lose information or introduce errors
• Error handling is missing or inconsistent
• The data shape changes without type safety

---

Step 4: Edge Case Identification

Systematically identify edge cases that may not be handled:

Input Edge Cases

• Null/undefined: What happens when optional fields are missing?
• Empty values: Empty strings, empty arrays, zero-length inputs
• Boundary values: Maximum lengths, integer overflow, date boundaries
• Invalid types: String where number expected, nested objects where flat expected
• Unicode/special characters: Emojis, RTL text, null bytes

Concurrency Edge Cases

• Race conditions: Two requests modifying the same resource simultaneously
• Stale reads: Reading data that was modified between read and write
• Deadlocks: Circular lock dependencies in database or mutex usage
• Retry storms: Failures causing exponential retry amplification

State Edge Cases

• Partial failures: What if step 2 of 3 fails? Is state consistent?
• Idempotency: Can the same operation be safely retried?
• Ordering: Does the system assume events arrive in order?
• Cold start: What happens on first run with no existing data?

---

Step 5: Test Matrix Generation

Based on the edge cases found, generate a test matrix:

## Test Matrix

| # | Area | Scenario | Input | Expected | Priority | Covered? |
|---|------|----------|-------|----------|----------|----------|
| 1 | Auth | Missing password field | {email: "[e-mail]"} | 400 error | High | No |
| 2 | API | Concurrent updates | Two PATCH requests | Last-write-wins or conflict | High | No |
| 3 | DB | Empty result set | Query for nonexistent user | Graceful empty response | Medium | Yes |
| 4 | Input | Unicode in name | {name: "Test emoji"} | Stored and displayed correctly | Low | No |

Mark each scenario as covered or not. Uncovered high-priority scenarios are findings.

---

Step 6: Scalability Assessment

Evaluate how the system will behave under growth:

Current Bottlenecks

• N+1 query patterns in database access
• Unbounded list queries (no pagination)
• Synchronous operations that could be async
• In-memory storage that won't survive restarts

Growth Scenarios

• 10x users: What breaks first?
• 10x data: Which queries become slow?
• 10x requests/second: Where does latency spike?

Recommendations

• What should be addressed now (before it becomes a problem)?
• What can wait (and what signal tells you it's time)?

---

Step 7: Technical Findings Output

## Engineering Review Findings

**Date:** {YYYY-MM-DD}
**Scope:** {what was reviewed}

### Architecture Assessment
{2-3 sentences on structural health}

### Findings

| # | Area | Risk | Finding | Recommendation | Priority |
|---|------|------|---------|----------------|----------|
| 1 | Architecture | HIGH | Circular dependency between auth and user modules | Extract shared types to common module | Now |
| 2 | Data Flow | MEDIUM | User input reaches DB without validation in /api/comments | Add zod schema validation | Now |
| 3 | Edge Case | HIGH | No handling for concurrent booking of same slot | Add optimistic locking or DB constraint | Now |
| 4 | Scalability | LOW | In-memory session store | Move to Redis when > 100 concurrent users | Later |

### Test Matrix
{from Step 5}

### Scalability Summary
{from Step 6}

Risk levels:

• HIGH: Will cause bugs, data issues, or outages. Address now.
• MEDIUM: Could cause issues under certain conditions. Address soon.
• LOW: Minor concern or future consideration. Track for later.

---

Completion

After the review:

"Engineering review complete. Found {N} findings ({X} high, {Y} medium, {Z} low). Run /s:build to start implementing fixes, or /s:plan to update the roadmap with these findings."

---

Rules

• NEVER review code style or formatting. That is /s:review (Aspect 2: Code Quality).
• ALWAYS trace at least one critical data flow end-to-end. This catches the most issues.
• ALWAYS generate a test matrix. Untested edge cases are the primary source of production bugs.
• ALWAYS assess scalability, even for small projects. It shapes early design decisions.
• If the codebase is too large to review entirely, focus on the most recent phase's changes and the core data paths.
• Be concrete in findings. "Architecture could be better" is not useful. "Module X has a circular dependency with module Y via import Z" is useful.
• Reference specific files and line numbers where possible.