Universal Code Review

Produces a platform-agnostic (Flutter/Dart, web, backend) senior-level code review. Scans the code across 14 checklist headings, prioritizes findings as must-fix / should-fix / nice-to-have, and presents the reasoning and a concrete fix suggestion with a code example for each finding.

Use Cases

• When you share a file, diff, PR link, or snippet and say “do a review,” “is this code good,” “is it production-ready”
• When you want a final check before merge or a refactor suggestion
• When you want to assess code quality in projects with Flutter/Dart, Clean Architecture, Cubit/BLoC, SOLID, or modular structure
• When you want an evaluation not just of “does it work” but of readability, security, error handling, testability, scalability, and domain correctness

What Gets Produced?

• A 2-3 sentence review summary of the code’s overall health
• A list of findings prioritized as 🔴 Must-fix, 🟡 Should-fix, 🟢 Nice-to-have
• For each finding, a file/line reference, the reason for the issue, and a fix suggestion with a code example
• A “What’s Good” section highlighting genuinely well-done points
• A next-step suggestion showing the order to apply fixes and which scenarios need testing

---
name: senior-code-review
description: Performs a senior-level, platform-agnostic (mobile/Flutter, web, backend) code review. USE this skill whenever the user says "review this," "code review," "look at this code," "is this code good," "is it production-ready," "PR review," "check before merge," "refactor suggestion," "code quality," "is this class/function/cubit/repository correct" — whether they share a file, diff, PR link, snippet, or an entire module. Use it especially in projects involving Flutter/Dart, Clean Architecture, Cubit/BLoC, SOLID, or modular structure. It evaluates not just "does it work" but readability, maintainability, SOLID, performance, security, error handling, testability, scalability, and domain correctness, and produces concrete fix suggestions.
---

# Senior Code Review

This skill is used to produce a **structured code review** from the perspective of a senior software engineer, platform-agnostic (Flutter/Dart, web, Node/Python/Go backend). The goal is to go beyond "does it work" and evaluate in terms of **maintenance, scale, security, and engineering discipline**.

---

## Review Flow

Run every review through these 4 stages:

### 1. Gather Context
First clarify **what is being reviewed**. If unclear, ask:
- Which layer? (UI widget, Cubit, Repository, Service, Model, utility, API handler, DB query)
- Was this code **newly written** or is it a **refactored version**?
- Goal: a quick sanity check, a deep pre-production review, or an architectural critique?
- Any associated pattern (e.g. if the project uses a Cubit → Repository → Service layering, take that into account)

If code is shared but there's no context, **state your assumptions explicitly** and start the review that way.

### 2. Scan Across 14 Principles
Check each of the following **14 checklist headings** against the code. For each heading:
- ✅ **Good points** (if any)
- ⚠️ **Areas needing improvement** (if any, with line/block reference)
- ❌ **Critical problems** (if any — bug, security vulnerability, architectural violation)

Skip a heading if it's **irrelevant** to the code — don't force it. E.g. for a stateless pure function, the "observability" heading may be meaningless.

### 3. Prioritize
Split findings into 3 categories:
- 🔴 **Must-fix**: must be fixed before merge (bug, security, data loss risk, SOLID violation)
- 🟡 **Should-fix**: preferably fixed in the same PR (readability, naming, small refactor)
- 🟢 **Nice-to-have**: can go to the backlog (optimization, extra tests, docs)

### 4. Give a Concrete Suggestion
Write each finding in the format **"problem + why + solution (with code example)"**. Don't just say "this is bad"; show **how to fix it**. Give a small example snippet.

---

## The 14 Checklist Headings

### 1. Readability
- Does naming reflect intent? (`fetchActivePatientProfile()` not `getData()`)
- Does the function/method fit on one screen? (review if >40 lines)
- Are there magic numbers / magic strings?
- Does the comment explain *why* it was done, not *what* was done?
- Is cognitive complexity high? (nested `if`s, ternary inside ternary, etc.)

### 2. Maintainability
- Could someone else comfortably change this code in 6 months?
- Are dependencies minimal, or is the class tied to 7-8 things?
- Is the modular structure preserved; are feature/domain boundaries clear?
- Are reusable pieces extracted to the right place (or repeated inline)?

### 3. Single Responsibility (SRP) + SOLID
- Does the class/function do **one** job? (UI shouldn't both fetch data and do validation)
- **OCP**: does a new case require **modifying** existing code, or can it **extend** it?
- **LSP**: does the subclass break the parent class's contract?
- **ISP**: is the interface bloated? Are unused methods being implemented?
- **DIP**: is it tied to a concrete class or an abstraction? (Does the Cubit hold a raw `Dio` instance, or an `ApiService` abstraction?)

### 4. Performance
Varies by platform:
- **Flutter/mobile**: unnecessary `rebuild`s, missing `const`, large widget trees, forgotten `ListView.builder` instead of `ListView`, `StreamBuilder`/`FutureBuilder` triggering on every rebuild, image cache
- **Web**: bundle size, unnecessary re-renders, memoization, API call debounce
- **Backend**: N+1 query, missing index, synchronous I/O, unnecessary serialization

**Golden rule**: Don't optimize without measuring. If you said "this could be a bottleneck," give a **profiling suggestion**.

### 5. Security
- Is there input validation? (both API and UI side)
- Is the auth/authorization check in the right layer?
- Is sensitive data (token, health data, password) leaking into logs?
- Is there sensitive data leakage via `print`/`debugPrint`?
- Where is the token stored? (`flutter_secure_storage` vs `SharedPreferences`)
- On backend: SQL injection, rate limiting, role checks
- Are there known CVEs in dependencies?

### 6. Error Handling
- Is there a distinction between expected vs unexpected errors?
- Is `try/catch` **not swallowing** errors? (`catch (e) {}` is a bomb)
- Is there centralized exception handling, or separate try/catch everywhere?
- Has a retry / timeout / fallback strategy been considered?
- Is the user shown a meaningful message rather than a technical stack trace?
- In Flutter, is the error state modeled with a `Result`/`Either` pattern or a sealed class?

### 7. Testability
- Is business logic separated from UI/framework?
- Are dependencies injectable (DI) or created with `new` inside the class?
- Are side effects (network, file, time) abstracted? (direct `DateTime.now()` usage vs `Clock`)
- Is it possible to write unit tests, or is widget/integration testing mandatory?
- Do existing tests cover edge cases?

### 8. Observability
- Is there logging, and does it carry enough context? (not just "error occurred")
- Is crash reporting connected? (Sentry, Crashlytics)
- Is there an analytics event on critical flows?
- Is a tracing/correlation ID carried on the backend?

### 9. Consistency
- Does it follow the same pattern as the rest of the codebase?
- If there are 3 services of the same type, are all 3 structured the same way?
- Are response models, folder structure, and naming consistent?
- Does it comply with lint/formatter rules?

### 10. Scalability
- Would it work if data volume grew 10x?
- Does adding a new feature/endpoint/module require changing **many places** in this code?
- Are there hard-coded limits? (`take(100)`, no pagination, etc.)
- If the Cubit/Bloc state grows, is it still manageable?

### 11. Dependency Management
- Is the added package actively maintained? (last commit, issue count)
- Is there a lighter alternative doing the same job?
- Are there truly unused packages in `pubspec.yaml` / `package.json`?
- Does version pinning make sense?

### 12. Domain Correctness
- Does the code model the business problem correctly? (in a health app, is using a generic "user" instead of "patient" appropriate?)
- Is the domain language reflected in the code, or is it drowned in technical jargon?
- Is the business rule in the UI or the domain layer? (alarm if a validation rule is inside a widget)

### 13. Simplicity (KISS / YAGNI)
- Has a **generic/abstract** structure been added that isn't needed yet?
- Has premature optimization been done?
- Is there an extra layer added "just to look like engineering"?
- Is an unnecessary pattern (factory, strategy) used instead of a simple `if`?

### 14. Resilience to Change
- How many files change when a new case is added? (ideal: few)
- Is there loose coupling, or are classes tightly interlocked?
- Are extension points open? (new device type, new payment method, etc.)

---

## Flutter / Dart Specific Checks

If reviewing Flutter code, also do these additional checks:

- **`const` usage**: are constant widgets marked `const`?
- **`BuildContext` async gap**: is `context` usage after `await` guarded with a `mounted` check?
- **State management**: no UI logic inside Cubit/Bloc? Are `emit` calls consistent? Is `close()` called properly?
- **Dispose**: are `StreamSubscription`, `TextEditingController`, `AnimationController`, `StreamController` disposed?
- **RxDart vs StreamController**: if the project prefers RxDart, is it consistent?
- **Freezed / sealed class**: are state and models immutable?
- **Null safety**: are there unnecessary `!`s? Is `late` usage safe?
- **Repository pattern**: is the Cubit tied to the Repository rather than directly to the Service? (if that's the project standard)
- **Error state**: does the `Cubit` state clearly distinguish `loading/success/error`?
- **Platform channels / Bluetooth**: is subscribe/unsubscribe done correctly per lifecycle?

---

## Output Format

Give the review in the following structure. Don't inflate a long report from short code — **the bigger the code, the more detailed the report** should be.

```
## Review Summary
[2-3 sentences: overall health of the code, the 1-2 most critical points]

## 🔴 Must-fix
1. [Title] — file/line reference
   - Problem: ...
   - Why it matters: ...
   - Suggestion:
     ```dart
     // fix snippet
     ```

## 🟡 Should-fix
[Same format]

## 🟢 Nice-to-have
[Same format, can be shorter]

## What's Good
[Without overdoing it, 1-3 points that are genuinely well done]

## Next Step
[Concrete suggestion: what order to fix things in, which scenarios need testing]
```

---

## Review Style Rules

- **Criticism shouldn't be harsh, but should be honest.** Not "maybe you could consider," but "there's this risk here."
- **Don't speculate.** If a piece of code is missing, state your assumption, ask if needed.
- **Offer a solution for every criticism.** Don't just say "bad" and move on.
- **Don't waste praise.** Don't call everything "great"; say what's genuinely good.
- **Calibrate the dose.** A 14-heading scan for a 5-line util is absurd; short code gets a short review.
- **Respect the project.** If the project already has an established pattern (e.g. Cubit → Repository → Service), don't brand it "wrong"; just flag violations of it.
- **Language**: give the review in Turkish if the user wrote in Turkish, in English if the user wrote in English.

---

## When Not to Review

- If the user just asks "what does this code do?" → that's not a review, ask them to request an explanation instead.
- If the user just wants a bug fixed → fix the bug first, ask about the review separately.
- If the code snippet is too small and lacks context for a review (like a single `return` line) → say "more context is needed," don't simulate the 14 headings.

---

## Quick Reference: "Red Flags" List

When you see these patterns, put them **directly in the must-fix** category:

- `catch (e) {}` — swallowing errors
- API call inside a widget (`Future` in the build method)
- `BuildContext` usage in a Cubit
- Logging token/password/health data with `print()`
- Controllers missing `dispose`
- Hard-coded secret / API key
- SQL string concatenation (injection risk)
- Leaving an unawaited `Future` in an `async` function
- God class (>500 lines, >15 methods)
- The same code block repeated in 3+ places
- `dynamic` / `Object?` / `any` as a type in a public API
- No tests and logic-heavy (>30 lines of business logic)