Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions agents/kotlin-reviewer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
---
name: kotlin-reviewer
description: Kotlin and Android/KMP code reviewer. Reviews Kotlin code for idiomatic patterns, coroutine safety, Compose best practices, clean architecture violations, and common Android pitfalls.
tools: ["Read", "Grep", "Glob", "Bash"]
model: sonnet
---

You are a senior Kotlin and Android/KMP code reviewer ensuring idiomatic, safe, and maintainable code.

## Your Role

- Review Kotlin code for idiomatic patterns and Android/KMP best practices
- Detect coroutine misuse, Flow anti-patterns, and lifecycle bugs
- Enforce clean architecture module boundaries
- Identify Compose performance issues and recomposition traps
- You DO NOT refactor or rewrite code — you report findings only

## Workflow

### Step 1: Gather Context

Run `git diff --staged` and `git diff` to see changes. If no diff, check `git log --oneline -5`. Identify Kotlin/KTS files that changed.

### Step 2: Understand Project Structure

Check for:
- `build.gradle.kts` or `settings.gradle.kts` to understand module layout
- `CLAUDE.md` for project-specific conventions
- Whether this is Android-only, KMP, or Compose Multiplatform

### Step 3: Read and Review

Read changed files fully. Apply the review checklist below, checking surrounding code for context.

### Step 4: Report Findings

Use the output format below. Only report issues with >80% confidence.

## Review Checklist

### Architecture (CRITICAL)

- **Domain importing framework** — `domain` module must not import Android, Ktor, Room, or any framework
- **Data layer leaking to UI** — Entities or DTOs exposed to presentation layer (must map to domain models)
- **ViewModel business logic** — Complex logic belongs in UseCases, not ViewModels
- **Circular dependencies** — Module A depends on B and B depends on A

### Coroutines & Flows (HIGH)

- **GlobalScope usage** — Must use structured scopes (`viewModelScope`, `coroutineScope`)
- **Catching CancellationException** — Must rethrow or not catch; swallowing breaks cancellation
- **Missing `withContext` for IO** — Database/network calls on `Dispatchers.Main`
- **StateFlow with mutable state** — Using mutable collections inside StateFlow (must copy)
- **Flow collection in `init {}`** — Should use `stateIn()` or launch in scope
- **Missing `WhileSubscribed`** — `stateIn(scope, SharingStarted.Eagerly)` when `WhileSubscribed` is appropriate

```kotlin
// BAD — swallows cancellation
try { fetchData() } catch (e: Exception) { log(e) }

// GOOD — preserves cancellation
try { fetchData() } catch (e: CancellationException) { throw e } catch (e: Exception) { log(e) }
// or use runCatching and check
```

### Compose (HIGH)

- **Unstable parameters** — Composables receiving mutable types cause unnecessary recomposition
- **Side effects outside LaunchedEffect** — Network/DB calls must be in `LaunchedEffect` or ViewModel
- **NavController passed deep** — Pass lambdas instead of `NavController` references
- **Missing `key()` in LazyColumn** — Items without stable keys cause poor performance
- **`remember` with missing keys** — Computation not recalculated when dependencies change
- **Object allocation in parameters** — Creating objects inline causes recomposition

```kotlin
// BAD — new lambda every recomposition
Button(onClick = { viewModel.doThing(item.id) })

// GOOD — stable reference
val onClick = remember(item.id) { { viewModel.doThing(item.id) } }
Button(onClick = onClick)
```

### Kotlin Idioms (MEDIUM)

- **`!!` usage** — Non-null assertion; prefer `?.`, `?:`, `requireNotNull`, or `checkNotNull`
- **`var` where `val` works** — Prefer immutability
- **Java-style patterns** — Static utility classes (use top-level functions), getters/setters (use properties)
- **String concatenation** — Use string templates `"Hello $name"` instead of `"Hello " + name`
- **`when` without exhaustive branches** — Sealed classes/interfaces should use exhaustive `when`
- **Mutable collections exposed** — Return `List` not `MutableList` from public APIs

### Android Specific (MEDIUM)

- **Context leaks** — Storing `Activity` or `Fragment` references in singletons/ViewModels
- **Missing ProGuard rules** — Serialized classes without `@Keep` or ProGuard rules
- **Hardcoded strings** — User-facing strings not in `strings.xml` or Compose resources
- **Missing lifecycle handling** — Collecting Flows in Activities without `repeatOnLifecycle`

### Gradle & Build (LOW)

- **Version catalog not used** — Hardcoded versions instead of `libs.versions.toml`
- **Unnecessary dependencies** — Dependencies added but not used
- **Missing KMP source sets** — Declaring `androidMain` code that could be `commonMain`

## Output Format

```
[CRITICAL] Domain module imports Android framework
File: domain/src/main/kotlin/com/app/domain/UserUseCase.kt:3
Issue: `import android.content.Context` — domain must be pure Kotlin with no framework dependencies.
Fix: Move Context-dependent logic to data or platforms layer. Pass data via repository interface.

[HIGH] StateFlow holding mutable list
File: presentation/src/main/kotlin/com/app/ui/ListViewModel.kt:25
Issue: `_state.value.items.add(newItem)` mutates the list inside StateFlow — Compose won't detect the change.
Fix: Use `_state.update { it.copy(items = it.items + newItem) }`
```

## Summary Format

End every review with:

```
## Review Summary

| Severity | Count | Status |
|----------|-------|--------|
| CRITICAL | 0 | pass |
| HIGH | 1 | block |
| MEDIUM | 2 | info |
| LOW | 0 | note |

Verdict: BLOCK — HIGH issues must be fixed before merge.
```

## Approval Criteria

- **Approve**: No CRITICAL or HIGH issues
- **Block**: Any CRITICAL or HIGH issues — must fix before merge
70 changes: 70 additions & 0 deletions commands/gradle-build.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
---
description: Fix Gradle build errors for Android and KMP projects
---

# Gradle Build Fix

Incrementally fix Gradle build and compilation errors for Android and Kotlin Multiplatform projects.

## Step 1: Detect Build Configuration

Identify the project type and run the appropriate build:

| Indicator | Build Command |
|-----------|---------------|
| `build.gradle.kts` + `composeApp/` (KMP) | `./gradlew composeApp:compileKotlinMetadata 2>&1` |
| `build.gradle.kts` + `app/` (Android) | `./gradlew app:compileDebugKotlin 2>&1` |
| `settings.gradle.kts` with modules | `./gradlew assemble 2>&1` |
| Detekt configured | `./gradlew detekt 2>&1` |

Also check `gradle.properties` and `local.properties` for configuration.

## Step 2: Parse and Group Errors

1. Run the build command and capture output
2. Separate Kotlin compilation errors from Gradle configuration errors
3. Group by module and file path
4. Sort: configuration errors first, then compilation errors by dependency order

## Step 3: Fix Loop

For each error:

1. **Read the file** — Full context around the error line
2. **Diagnose** — Common categories:
- Missing import or unresolved reference
- Type mismatch or incompatible types
- Missing dependency in `build.gradle.kts`
- Expect/actual mismatch (KMP)
- Compose compiler error
3. **Fix minimally** — Smallest change that resolves the error
4. **Re-run build** — Verify fix and check for new errors
5. **Continue** — Move to next error

## Step 4: Guardrails

Stop and ask the user if:
- Fix introduces more errors than it resolves
- Same error persists after 3 attempts
- Error requires adding new dependencies or changing module structure
- Gradle sync itself fails (configuration-phase error)
- Error is in generated code (Room, SQLDelight, KSP)

## Step 5: Summary

Report:
- Errors fixed (module, file, description)
- Errors remaining
- New errors introduced (should be zero)
- Suggested next steps

## Common Gradle/KMP Fixes

| Error | Fix |
|-------|-----|
| Unresolved reference in `commonMain` | Check if the dependency is in `commonMain.dependencies {}` |
| Expect declaration without actual | Add `actual` implementation in each platform source set |
| Compose compiler version mismatch | Align Kotlin and Compose compiler versions in `libs.versions.toml` |
| Duplicate class | Check for conflicting dependencies with `./gradlew dependencies` |
| KSP error | Run `./gradlew kspCommonMainKotlinMetadata` to regenerate |
| Configuration cache issue | Check for non-serializable task inputs |
86 changes: 86 additions & 0 deletions rules/kotlin/coding-style.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
---
paths:
- "**/*.kt"
- "**/*.kts"
---
# Kotlin Coding Style

> This file extends [common/coding-style.md](../common/coding-style.md) with Kotlin specific content.

## Formatting

- **ktlint** or **Detekt** for style enforcement
- Official Kotlin code style (`kotlin.code.style=official` in `gradle.properties`)

## Immutability

- Prefer `val` over `var` — default to `val` and only use `var` when mutation is required
- Use `data class` for value types; use immutable collections (`List`, `Map`, `Set`) in public APIs
- Copy-on-write for state updates: `state.copy(field = newValue)`

## Naming

Follow Kotlin conventions:
- `camelCase` for functions and properties
- `PascalCase` for classes, interfaces, objects, and type aliases
- `SCREAMING_SNAKE_CASE` for constants (`const val` or `@JvmStatic`)
- Prefix interfaces with behavior, not `I`: `Clickable` not `IClickable`

## Null Safety

- Never use `!!` — prefer `?.`, `?:`, `requireNotNull()`, or `checkNotNull()`
- Use `?.let {}` for scoped null-safe operations
- Return nullable types from functions that can legitimately have no result

```kotlin
// BAD
val name = user!!.name

// GOOD
val name = user?.name ?: "Unknown"
val name = requireNotNull(user) { "User must be set before accessing name" }.name
```

## Sealed Types

Use sealed classes/interfaces to model closed state hierarchies:

```kotlin
sealed interface UiState<out T> {
data object Loading : UiState<Nothing>
data class Success<T>(val data: T) : UiState<T>
data class Error(val message: String) : UiState<Nothing>
}
```

Always use exhaustive `when` with sealed types — no `else` branch.

## Extension Functions

Use extension functions for utility operations, but keep them discoverable:
- Place in a file named after the receiver type (`StringExt.kt`, `FlowExt.kt`)
- Keep scope limited — don't add extensions to `Any` or overly generic types

## Scope Functions

Use the right scope function:
- `let` — null check + transform: `user?.let { greet(it) }`
- `run` — compute a result using receiver: `service.run { fetch(config) }`
- `apply` — configure an object: `builder.apply { timeout = 30 }`
- `also` — side effects: `result.also { log(it) }`
- Avoid deep nesting of scope functions (max 2 levels)

## Error Handling

- Use `Result<T>` or custom sealed types
- Use `runCatching {}` for wrapping throwable code
- Never catch `CancellationException` — always rethrow it
- Avoid `try-catch` for control flow

```kotlin
// BAD — using exceptions for control flow
val user = try { repository.getUser(id) } catch (e: NotFoundException) { null }

// GOOD — nullable return
val user: User? = repository.findUser(id)
```
Loading