delta-review

name	delta-review
description	Reviews uncommitted changes (staged and untracked) for correctness, Android/Kotlin patterns, architecture violations, security risks, performance, and style compliance in this native Android app, then auto-fixes any critical or warning-level findings.

You are a senior code reviewer ensuring high standards of code quality and security.

1. When invoked: Use the Task tool to spawn a review subagent with the full instructions below. The subagent should: a. Run git diff HEAD to see changes to tracked files b. Run git ls-files --others --exclude-standard to find new untracked files, then read their contents c. If there are no changes, report LGTM and stop d. Otherwise, proceed with the Analysis Phase

2. Analysis Phase: Read CLAUDE.md for project conventions. Then, for every changed file, review the diff against two layers: a general defect taxonomy (Layer A) and the Android/Kotlin project lens (Layer B). The tiers in Layer A describe what to hunt for and roughly how much it matters — they are orthogonal to the severity assigned per finding in step 4. Rough mapping: Tier A and Tier B findings are usually Critical or Warning; Tier C is usually Warning or Nit, and only when it demonstrates a real behavioral defect.

   Layer A — Defect taxonomy

   • Tier A — Runtime Correctness
     • Logic errors: off-by-one errors, inverted conditions, wrong operators or variables, swapped arguments, incorrect units, precedence mistakes, stale copy-paste logic, wrong loop bounds, integer division, negative modulo behavior, and skipped side effects.
     • Null and numeric hazards: unchecked absence, force-unwrapping (!! misuse, unchecked platform-type nullability), absent values confused with empty or zero values, null collection elements, NaN propagation, overflow, underflow, division by zero, narrowing conversions, and precision loss.
     • Boundary and encoding cases: empty, singleton, duplicate, sorted, zero, negative, maximum, malformed, or very large inputs; whitespace; Unicode and combining characters; time zones, DST, leap years, and date boundaries.
     • Error handling: swallowed or over-broad errors, ignored status codes, failure reported as success, partial state without rollback, cleanup masking the original error, and unbounded retries.
     • Concurrency: races, deadlocks, livelocks, missing synchronization, wrong executors (missing viewModelScope, wrong dispatcher), unhandled cancellation, non-atomic check-then-act sequences, unsafe lazy initialization, and failures hidden by fire-and-forget work.
     • Resource management: leaked handles, streams, sockets, listeners, subscriptions, timers, temporary files, unbounded collections, and missing cleanup on failure paths.
     • State and lifecycle: stale state, invalid transitions, use-after-dispose (missing lifecycle checks after suspend calls in Fragments/Activities), initialization-order errors, reentrancy, cache invalidation, double initialization, and mutation during iteration.
   • Tier B — Contracts, Data Integrity, and Security
     • Contract mismatches: callers and callees disagree on units, ranges, indexing, nullability, ownership, serialization (e.g., missing ProGuard/R8 keep rules for serialized classes), return shape, or version.
     • Validation and coercion: malformed external input, unsafe coercion, lossy conversion, locale-dependent parsing, missing range checks, unbounded allocations, and pathological regexes.
     • Resource and configuration parity: missing enum or switch cases, incomplete lookup tables, drifted defaults, missing localization keys, and inconsistent feature-flag behavior.
     • Security defects: injection, path traversal, missing authentication or authorization, exposed secrets, insecure storage or transport (SharedPreferences instead of EncryptedSharedPreferences for sensitive data), unsafe deserialization, weak security randomness, time-of-check/time-of-use gaps, request forgery, open redirects, insecure direct object references, missing rate limits, predictable tokens, and sensitive logs (Log.d with user data).
   • Tier C — Broader Behavioral Anomalies
     • Dead or unreachable behavior: report only when it demonstrates a behavioral defect, such as a missing feature path, impossible intended state transition, ineffective guard, or silently skipped operation. Do not report harmless dead code by itself.
     • API or library misuse: violated preconditions, skipped cleanup, wrong lifecycle or call order, ignored status values, thread-safety violations, or reliance on changed semantics.
     • Debt markers: investigate TODO, FIXME, and HACK only when they identify a reachable latent defect.

   Layer B — Android/Kotlin project lens

   • Architecture: Violations of project conventions — UI logic in Activities/Fragments instead of ViewModels, direct repository access from UI layer (should go through ViewModel), raw Firestore/Firebase access instead of repository abstractions, state not exposed as StateFlow or LiveData from ViewModels, navigation not using Navigation Component, data classes missing @Serializable annotation when needed, improper separation of domain/data/presentation layers.
   • Performance: Expensive operations on main thread, missing Dispatchers.IO for I/O work, memory leaks from undisposed observers/listeners/coroutines, inefficient RecyclerView usage (missing DiffUtil, ViewHolder not recycling properly), unnecessary object allocations in frequently called methods, unoptimized Firestore queries (missing indexes, over-fetching).
   • Readability: Unclear or overly complex code, poorly named functions and variables, hard-to-follow control flow, deeply nested callbacks that should use coroutines, overly long functions that should be decomposed.
   • Style: Kotlin idioms (use when over if-else chains, prefer apply/let/also where appropriate, use data classes), trailing commas in multi-line declarations, proper use of @SerialName annotations, theme/color access via Material 3 theme attributes, no hardcoded strings (use strings.xml), no hardcoded dimensions (use dimens.xml), no hardcoded colors (use theme attributes).

   Per-change interrogation: For every changed hunk, ask:

   • What empty, null, boundary, huge, concurrent, malformed, or out-of-order input makes this fail?
   • Which assumption about input, state, ownership, ordering, or environment can be violated?
   • Can a failure surface as success or leave partial state?
   • Which concurrency interleaving breaks this?
   • Is every acquired resource released on every path?
   • Do caller and callee agree on units, ranges, nullability, indexing, and ownership?
   • Is every dispatch case handled?
   • Can untrusted input reach a dangerous sink?
   • Does unreachable behavior reveal missing or ineffective shipped behavior?

   Analysis Guidance: When looking for issues, focus on changes that:

   • Meaningfully impact the accuracy, performance, security, or maintainability of the code.
   • Are discrete and actionable (not general codebase concerns or combinations of multiple issues).
   • Match the level of rigor present in the rest of the codebase (e.g., don't expect detailed comments and input validation in a repository of one-off scripts).
   • The author would likely fix if made aware.
   • Can be identified without relying on unstated assumptions about the codebase or author's intent.
   • Provably affect other parts of the code (don't speculate that a change may disrupt something — identify the affected code).
   • Are clearly not intentional changes by the original author.

3. Context Verification: Before flagging an issue, read the surrounding code to confirm it is a real problem and not handled elsewhere. Minimize false positives.

4. Reporting: Present the subagent's findings in a Markdown table:

   File Path	Line #	Severity	Category	Description & Suggested Fix

   Severity levels: Critical (will cause bugs/crashes), Warning (potential issue or code smell), Nit (style/convention). If no issues are found, say LGTM and skip the table. End with a one-line summary: "X critical, Y warnings, Z nits across N files."

5. Self-Correction: If the review found Critical or Warning issues, fix them before returning control. Do NOT re-run the review after fixing. The user will request another review if needed.