Flutter Code Review

Flutter Code Review Skill

Purpose

Synthesize findings from across all Flutter engineering domains into a prioritized, actionable review with clear severity classification — not a list of style nits.

Scope and authority

This skill is the lead authority for:

• PR review and code quality assessment
• tech debt severity classification
• cross-domain finding synthesis
• review finding prioritization
• refactoring sequencing recommendations
• standards enforcement reporting

This skill activates domain skills for deep-domain findings:

• Security violations → escalate to flutter-security (may trigger override)
• Architecture violations → flutter-architecture
• Performance anti-patterns → flutter-performance
• Test gaps → flutter-testing

Review severity rubric

Review checklist (systematic, per domain)

1. Architecture and layering

□ No business logic in widget build() methods
□ No repositories or data sources accessed from UI layer
□ Feature-first structure maintained (no global screens/models/services)
□ No circular dependencies between packages/features
□ No feature-to-feature direct imports
□ Domain models are pure Dart (no Flutter framework imports)
□ DI wiring via Riverpod providers (no manual instantiation in widgets)
□ No GetX usage

2. State management

□ ref.watch() only in build() — not in callbacks
□ ref.read() used in event handlers / onPressed
□ select() used when only part of state is needed
□ No setState() for shared/app state
□ Providers defined in provider files (not inside widgets)
□ AsyncNotifier used for async operations (not manual isLoading bool)
□ No global mutable singletons
□ Providers properly overridden in tests

3. Flutter-specific

□ const constructors used wherever applicable
□ ListView.builder used for all lists (not Column + map + toList)
□ FutureBuilder not recreated in build() — future assigned in initState or state notifier
□ mounted check after every async operation using BuildContext
□ dispose() implemented for all controllers (animation, text, scroll, stream subscriptions)
□ No print() statements in production code
□ RepaintBoundary used for expensive, independently-repainting subtrees
□ No IntrinsicHeight/Width in scroll hot paths
□ No Opacity widget for decoration (use withOpacity on Color)
□ No ClipRRect used decoratively (use BoxDecoration.borderRadius)

4. Security

□ No tokens/PII in SharedPreferences → flutter_secure_storage only
□ No API keys or secrets in source code
□ No sensitive data in log statements (even debug logs)
□ Dio interceptors do not log Authorization headers
□ No HTTP cleartext in production config
□ --obfuscate flag documented in release instructions
□ No dynamic that could mask type safety violations

5. Networking

□ All HTTP calls go through repository → remote data source (not directly in UI)
□ Auth interceptor handles token refresh transparently
□ Retry interceptor with backoff in place
□ CancelToken used for search/type-ahead requests
□ Timeout configured on Dio (not unbounded)
□ Logging interceptor disabled in release mode
□ No raw dynamic parsing of API responses (use fromJson with typed models)

6. Async safety

□ No BuildContext used after await without mounted check
□ No unawaited futures silently discarded (use unawaited() explicitly or await)
□ No async void methods except for top-level event handlers
□ Timer.periodic cancelled in dispose()
□ StreamSubscription.cancel() called in dispose()
□ No isolate spawned without error handler

7. Testing

□ New features have unit tests for domain/application layer
□ New screens have widget tests
□ FakeRepository used (not mocked repository) for stateful test scenarios
□ No DateTime.now() in testable code (inject Clock)
□ No network calls in unit/widget tests
□ Golden tests updated when design changes
□ Patrol test added for new critical journey (or tech debt tracked)

8. Performance

□ No MediaQuery.of(context) in leaf widgets (causes unnecessary parent rebuilds)
□ No ItemBuilder recreating expensive widgets without caching
□ Isolate.run() used for heavy computation (JSON decode, image processing)
□ Image loading uses cached_network_image or ResizeImage
□ No unnecessary AnimationController created without disposal

Review output format

## Code Review Summary

### Critical (block merge)
[findings]

### High (must fix before merge)
[findings]

### Medium (fix this sprint)
[findings]

### Low / Nitpick (author discretion)
[findings]

### Positive findings (reinforce good patterns)
[findings]

### Suggested refactoring sequence
[prioritized next steps beyond this PR]

Anti-pattern detection across review

Automatically flag any of the global anti-patterns from ../../PROJECT_RULES.md.

Additionally:

• Test coverage reduction in critical flows (compared to baseline)
• PR scope creep (mixing unrelated concerns — suggest splitting)
• Temporary compromises without comments, TODOs, or tracking tickets
• Inconsistent naming patterns breaking codebase conventions
• Missing error handling on user-visible async operations

Cross-domain escalation rules

Uncertainty protocol

• High (≥ 0.80): full code context available, requirements known
• Medium (0.60–0.79): partial context or unclear requirements
• Low (< 0.60): reviewing code without understanding domain

Cross-skill handoff payload

Use the standard payload from ../../AGENTS.md. Set requesting_skill to flutter-code-review.

Output contract

Return findings in severity order. Also include:

• Summary: severity counts
• Findings by severity with code location + evidence + fix
• Cross-skill escalations
• Positive patterns to reinforce
• Suggested refactoring sequence

Related resources

• references/review-checklist.md
• references/anti-patterns.md
• templates/review-report.md