Skip to main content

REVIEW.md — [App name, e.g. Dw2ni]

Check against the PR's story and acceptance scenarios​

Read the PR description's "Story" and "Acceptance Scenarios (Given-When-Then)" sections before reviewing the diff.

  • Flag any listed acceptance scenario with no code path in the diff that satisfies it — Important, name the scenario specifically.
  • Flag diffs that contradict a listed scenario, quoting it.
  • If the section is empty, report once as a nit; don't guess intent.

Escalate to Important​

  • Secrets and credentials. Any API key, token, or credential hardcoded in Dart source, pubspec.yaml, or committed config, instead of read from [CONFIRM: .env + flutter_dotenv / native build config / a secrets manager]. Never flag false positives on test/mock keys clearly scoped to a test/ directory.
  • Insecure local storage of sensitive data. Auth tokens, PII, or payment-adjacent data written to SharedPreferences or plain files instead of flutter_secure_storage (iOS Keychain / Android Keystore). SharedPreferences is fine for non-sensitive app state (theme, locale) — don't flag those.
  • State management inconsistency. [CONFIRM the app's chosen approach: Provider / Riverpod / Bloc / GetX / other] — flag new screens or features that introduce a different pattern than the rest of the app without a stated reason in the PR description.
  • Unhandled Future/async errors. An async call with no try/catch (or unguarded .catchError) on a path that can plausibly fail — network calls, file I/O, platform channel calls — especially where a failure would leave the UI in a stuck loading state with no user-visible error or retry.
  • Platform-channel calls with no error handling. MethodChannel invocations that don't handle PlatformException — a native-side failure should not crash the Dart isolate silently.
  • Blocking the UI thread with heavy work. Synchronous JSON parsing, image processing, or large-list sorting done inline on a widget build path instead of via compute()/an isolate, for payloads large enough to plausibly cause jank.

Report as-is (default severity)​

  • Missing const constructors on widgets that could be const — real performance impact (avoids unnecessary rebuilds) but rarely worth blocking a PR over.
  • New screens or widgets with no corresponding widget test.
  • API response models that don't handle a null/missing field defensively (should default or throw a clear error, not silently produce a broken UI state).
  • Missing RTL/localization handling on new user-facing strings — [CONFIRM: are Arabic/RTL layouts required for this app the way they are for the web verticals? If yes, treat as Important instead.]

Nit only​

  • Formatting/lint issues already caught by flutter analyze / dart format — don't duplicate what CI already reports.
  • Minor naming inconsistencies with no functional effect.

Skip​

  • *.md, docs/**.
  • Generated files: *.g.dart, *.freezed.dart, l10n/generated/**, anything under build/.

Nit volume​

At most five nits per review; mention any remainder as a count in the summary.

Open items to confirm before first real use​

  • State management library in use.
  • Networking layer (Dio / http / other) and its standard error- handling wrapper, if one exists.
  • Local persistence choice for structured data (sqlite/Drift, Hive, Isar) vs. simple key-value (SharedPreferences/secure storage).
  • Whether this app shares a backend/API contract with any kernel engine (IAM, Payment, etc.) or is fully independent.
  • Minimum supported Flutter/Dart SDK version, so version-gated API usage can be checked accurately.

Notes for reviewers​

Findings are informational — Code Review does not approve or block PRs. Review Behavior: Once after PR creation.