diff --git a/CHANGELOG.md b/CHANGELOG.md index d68a597..87f8e2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ Full release history for the Compose Skill Suite. The newest release is summarised under **What's new** in the [README](./README.md). +### 4.3.0 — 2026-07-06 + +**`jetpack-compose-audit` only — cross-phase back-write detection + false-lead guard.** + +- **Cross-phase back-writes — axis 3 (new detector).** The audit already deducted for same-body backwards writes; it now also catches the subtler cross-phase shape — a **layout-phase** write (`onSizeChanged` / `onGloballyPositioned` / `onPlaced`) into state a sibling reads in composition. `search-playbook.md` §3 gains the heuristic, `scoring.md` the matching Performance deduction (Critical when the loop is per-frame or spans reused lazy items), `diagnostics.md` a step-7 triage. Findings cite the writer and reader `file:line`. +- **Composition-phase self-invalidation (related check).** A `SnapshotStateMap` / `SnapshotStateList` (including `toMutableStateList()` / `toMutableStateMap()` builders) mutated inside a `@Composable` body that also reads it. This is *not* cross-phase — write and read are both in composition — and is deduped against the same-body backwards-write rule so it never double-counts. +- **False-lead guard (scoring correctness).** `scoring.md` Performance gains a **Do Not Credit — False Leads** table: plausible "recomposition fixes" that change nothing (`remember(index)` on pure functions, identity caches for derived maps, layout modifiers on both measured and sibling rows, `Exactly(1)` forced on both rows, and — under Strong Skipping — manually `remember`-ing a callback that the compiler already auto-memoizes). The auditor no longer rewards these and will not emit them in `Prioritized Fixes` — it verifies against runtime counts / compiler reports instead of the presence of the pattern. +- **Attribution.** The three-axis framing and the false-lead cases are adapted (reworded, re-cited against `developer.android.com`) from [`chrisbanes/skills`](https://github.com/chrisbanes/skills) `compose-recomposition-performance` (Apache-2.0). Axes 1 (stability/skipping) and 2 (deferred reads) were already covered by this suite; this release imports only axis 3 and the false-lead guard. +- **Versions.** `jetpack-compose-audit` → `4.3.0`. `compose-agent` unchanged at `4.2.1`. Note: the 4.2.2 Nav3 work was docs-only and never bumped the manifests, so `jetpack-compose-audit`'s `plugin.json` / SKILL version moves straight from `4.2.0` to `4.3.0` here — 4.2.2 lives in the changelog and shipped content but not in a version string. + ### 4.2.2 — 2026-06-30 **`jetpack-compose-audit` only — Navigation 3 audit patterns.** @@ -10,7 +20,7 @@ Full release history for the Compose Skill Suite. The newest release is summaris - **Scoring table.** `scoring.md` maps all Nav3 findings to existing categories: State Management (anonymous keys, ViewModel ownership, `ResultEventBus` risk) and Side Effects (composition-body navigation, missing `dropUnlessResumed`); `@Composable`-in-NavKey and Nav2+Nav3 conflict promoted to Critical Findings. - **Canonical sources.** Added `https://developer.android.com/guide/navigation/navigation-3` and `https://github.com/android/nav3-recipes` to `canonical-sources.md`. - **SKILL.md trigger.** Step 5 now directs auditors to run playbook section 2b whenever `rememberNavBackStack`, `NavDisplay`, or `NavKey` is detected. -- **Versions.** `jetpack-compose-audit` → `4.2.2`. `compose-agent` unchanged at `4.2.1`. +- **Versions.** `jetpack-compose-audit` → `4.2.2` (changelog/content only; the `plugin.json` / SKILL version strings stayed at `4.2.0` and are bumped to `4.3.0` in the next release). `compose-agent` unchanged at `4.2.1`. ### 4.2.1 — 2026-06-29 diff --git a/README.md b/README.md index 1e0b675..85f65fc 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ Claude Code plugin

-**`compose-agent` 4.2.1 · 2026-06-29** — Navigation 3 "when to use" decision table in `navigation.md` (workflow entry point: Nav3 vs Nav2 type-safe vs plain state, adaptive scene strategies; the back stack is mutated in the route, not a screen ViewModel; results via Nav3's `ResultEventBus`). Verified against the official `android/nav3-recipes` samples. `jetpack-compose-audit` stays at `4.2.0`. +**`jetpack-compose-audit` 4.3.0 · 2026-07-06** — Cross-phase back-write detection (axis 3: layout callbacks writing state read in composition), a related composition-phase self-invalidation check (snapshot collections mutated in a composable body), and a **False Leads** scoring guard so the auditor stops crediting no-op "recomposition fixes." Adapted from [`chrisbanes/skills`](https://github.com/chrisbanes/skills) (Apache-2.0). `compose-agent` stays at `4.2.1`. **Version 4.2.0 · 2026-06-17** — Paging 3 in Compose: new `paging.md` reference (LLM guardrails, not API tour), audit hooks under existing Performance/State categories, planning doc at [`docs/paging-skill-plan.md`](./docs/paging-skill-plan.md). Validated through multi-agent cross-review. Both skills ship as `4.2.0`. @@ -29,6 +29,18 @@ Authored and cross-reviewed with every frontier model — Claude Opus 4.8, GPT-5 ## What's new +### 4.3.0 — 2026-07-06 + +**`jetpack-compose-audit` — cross-phase back-writes + false-lead guard.** + +- **Cross-phase back-write detector (axis 3).** Beyond same-body backwards writes, the audit now flags a **layout-phase** write into state an earlier composition read: `onSizeChanged` / `onGloballyPositioned` / `onPlaced` writing state a sibling reads in composition. New heuristic in [`search-playbook.md`](./skills/jetpack-compose-audit/references/search-playbook.md), matching Performance deduction in [`scoring.md`](./skills/jetpack-compose-audit/references/scoring.md), triage step 7 in [`diagnostics.md`](./skills/jetpack-compose-audit/references/diagnostics.md). Findings cite writer and reader `file:line`. +- **Composition-phase self-invalidation (related, not cross-phase).** A `SnapshotStateMap` / `SnapshotStateList` mutated inside a `@Composable` body that also reads it. Deduped against the same-body backwards-write rule so it never double-counts. +- **False Leads guard.** New **Do Not Credit — False Leads** table in `scoring.md`: plausible recomposition "fixes" that change nothing. The auditor no longer rewards them and will not suggest them in `Prioritized Fixes`. +- **Attribution.** Axis 3 and the false-lead cases adapted (reworded, re-cited against `developer.android.com`) from [`chrisbanes/skills`](https://github.com/chrisbanes/skills) `compose-recomposition-performance` (Apache-2.0). Axes 1–2 were already covered by this suite. +- **Versions.** `jetpack-compose-audit` → `4.3.0`. `compose-agent` unchanged at `4.2.1`. + +For release detail, see [`docs/release-notes-4.3.0.md`](./docs/release-notes-4.3.0.md). Full history: [CHANGELOG.md](./CHANGELOG.md). + ### 4.2.1 — 2026-06-29 **`compose-agent` — Navigation 3 decision table.** @@ -68,7 +80,7 @@ Four categories, weighted for an app repo. Each scored `0-10`; overall on `0-100 | Category | Weight | What it covers | |----------|--------|----------------| -| **Performance** | 35% | Work in composition, lazy-list keys, state-read timing, stability, Strong Skipping, backwards writes, **animation phase correctness**, baseline profiles | +| **Performance** | 35% | Work in composition, lazy-list keys, state-read timing, stability, Strong Skipping, backwards writes, **cross-phase back-writes**, **animation phase correctness**, baseline profiles | | **State management** | 25% | Hoisting, single source of truth, `rememberSaveable`, lifecycle-aware collection, observable collections, ViewModel placement, type-safe navigation | | **Side effects** | 20% | Effect API choice, keys, stale captures, cleanup, composition-time work, **animation driving via `LaunchedEffect`** | | **Composable API quality** | 20% | Modifier conventions, parameter order, slot APIs, `CompositionLocal` usage, `Modifier.Node`, **`animationSpec` exposure**, `@Preview` coverage, hardcoded strings / magic numbers | diff --git a/docs/release-notes-4.3.0.md b/docs/release-notes-4.3.0.md new file mode 100644 index 0000000..bcba529 --- /dev/null +++ b/docs/release-notes-4.3.0.md @@ -0,0 +1,41 @@ +# Release notes — 4.3.0 (2026-07-06) + +**`jetpack-compose-audit` only.** `compose-agent` unchanged at `4.2.1`. + +Two additions to the Performance category, plus a scoring-correctness guard. The suite already covered axes 1 (stability/skipping) and 2 (deferred reads) of the recomposition problem; this release adds axis 3 and stops the auditor from crediting no-op "fixes". + +## Cross-phase back-writes (axis 3) + +A **layout-phase** callback (`onSizeChanged` / `onGloballyPositioned` / `onPlaced`) that writes snapshot state a sibling reads in **composition** creates a measure → write → recompose loop, often per-frame during resize/scroll. The audit already flagged same-body backwards writes; it now catches this cross-phase shape too. + +- `search-playbook.md` — new **Cross-Phase Back-Write & Composition Self-Invalidation Heuristic**: gather layout-callback writers, confirm a composition-phase reader, cite both `file:line`. +- `scoring.md` — Performance deduction; promoted to a Critical Finding when the loop is per-frame or spans reused lazy items. A matching Reward bullet credits measured values kept in layout/draw (`Modifier.layout { }` / `Modifier.drawBehind { }`). +- `diagnostics.md` — Quick Triage Recipe step 7 surfaces candidates before rubric reading, scoped to the files that actually hold snapshot collections. + +## Composition-phase self-invalidation (related check) + +A `SnapshotStateMap` / `SnapshotStateList` (including the `toMutableStateList()` / `toMutableStateMap()` builders) mutated inside a `@Composable` body that also reads it. This is **not** cross-phase — write and read are both in composition — and is **deduped** against the same-body backwards-write rule so it is scored once, never double-counted. + +## Do Not Credit — False Leads + +A guard table in `scoring.md` Performance: plausible "recomposition fixes" that change nothing, so the auditor neither rewards them nor emits them in `Prioritized Fixes`. + +- `remember(index) { … }` around a pure, cheap function of its own key. +- Identity/instance caches for read-only derived maps (use `remember(keys)`). +- Layout modifiers on both the measured and the sibling row (the sibling still reads size in composition). +- `assertRecompositionCount(Exactly(1))` forced on both rows when one correctly recomposes 0 times. +- Under **Strong Skipping**, manually `remember`-ing a callback the compiler already auto-memoizes — that lever only applies SSM-off, on `@NonSkippableComposable` / `@DontMemoize` paths, or when the lambda captures an unstable value. + +The guard verifies against runtime recomposition counts / compiler reports rather than the presence of a pattern. + +## Evals + +`evals/evals.json` cases 12–14 lock in the new behavior: cross-phase layout→composition back-write (12), false leads that must not be credited or suggested under SSM (13), and composition-phase snapshot self-invalidation (14). Audit-only — `compose-agent` has no cross-phase / false-lead guidance yet (tracked as follow-up). + +## Attribution + +Axis 3 and the false-lead cases are adapted — reworded and re-cited against `developer.android.com` — from [`chrisbanes/skills`](https://github.com/chrisbanes/skills) `compose-recomposition-performance` (Apache-2.0). + +## Versions + +`jetpack-compose-audit` → **4.3.0** (SKILL + both `plugin.json`). The 4.2.2 Nav3 work was changelog/content only and never bumped the manifests, so the version string moves from `4.2.0` straight to `4.3.0`. diff --git a/skills/jetpack-compose-audit/.claude-plugin/plugin.json b/skills/jetpack-compose-audit/.claude-plugin/plugin.json index 8b08506..55ee7af 100644 --- a/skills/jetpack-compose-audit/.claude-plugin/plugin.json +++ b/skills/jetpack-compose-audit/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "jetpack-compose-audit", - "version": "4.2.0", + "version": "4.3.0", "description": "Strict, evidence-based audit for Android Jetpack Compose repositories. Produces a 0-100 score, per-category 0-10 scores, a COMPOSE-AUDIT-REPORT.md with every deduction cited against developer.android.com, and coverage notes for animation performance, paging list correctness, UI tests, focus/keyboard, Compose Multiplatform, and Android launch UX surfaces.", "author": { "name": "Ivan Morgillo" diff --git a/skills/jetpack-compose-audit/.cursor-plugin/plugin.json b/skills/jetpack-compose-audit/.cursor-plugin/plugin.json index 8b08506..55ee7af 100644 --- a/skills/jetpack-compose-audit/.cursor-plugin/plugin.json +++ b/skills/jetpack-compose-audit/.cursor-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "jetpack-compose-audit", - "version": "4.2.0", + "version": "4.3.0", "description": "Strict, evidence-based audit for Android Jetpack Compose repositories. Produces a 0-100 score, per-category 0-10 scores, a COMPOSE-AUDIT-REPORT.md with every deduction cited against developer.android.com, and coverage notes for animation performance, paging list correctness, UI tests, focus/keyboard, Compose Multiplatform, and Android launch UX surfaces.", "author": { "name": "Ivan Morgillo" diff --git a/skills/jetpack-compose-audit/SKILL.md b/skills/jetpack-compose-audit/SKILL.md index 60abc34..2180a95 100644 --- a/skills/jetpack-compose-audit/SKILL.md +++ b/skills/jetpack-compose-audit/SKILL.md @@ -9,7 +9,7 @@ argument-hint: "[repo path or module path]" This skill audits Android Jetpack Compose repositories with a strict, evidence-based report. -**Skill version:** 4.2.0 — released 2026-06-17. **Compose track:** Kotlin 2.0.20+ / Compose Compiler 1.5.4+ (Strong Skipping Mode default). See the README changelog for what changed. +**Skill version:** 4.3.0 — released 2026-07-06. **Compose track:** Kotlin 2.0.20+ / Compose Compiler 1.5.4+ (Strong Skipping Mode default). See the README changelog for what changed. It is intentionally focused on four categories: @@ -206,7 +206,7 @@ Focus on: - paging list keys and `LazyPagingItems` misuse when `collectAsLazyPagingItems` is present - bad state-read timing - unstable or overly broad reads -- backwards writes +- backwards writes, including cross-phase back-writes (layout callback writing state read in composition) and snapshot-collection self-invalidation in a composable body - animation phase correctness (per-frame values deferred to layout/draw via lambda modifiers, `Animatable` held in `remember`, `rememberInfiniteTransition` scoped so it stops offscreen) - obvious release-performance hygiene where visible diff --git a/skills/jetpack-compose-audit/evals/evals.json b/skills/jetpack-compose-audit/evals/evals.json index 269e1fc..2286ad9 100644 --- a/skills/jetpack-compose-audit/evals/evals.json +++ b/skills/jetpack-compose-audit/evals/evals.json @@ -1,6 +1,6 @@ { "skill_name": "jetpack-compose-audit", - "description": "Acceptance evals for the API-hygiene, effect-correctness, cancellation, and state-boundary behavior introduced in 4.1.x, plus Paging 3 in Compose list/load-state behavior added in 4.2.0 (cases 10-11). These are the audit/review acceptance criteria the skill must preserve; they also back compose-agent review behavior. Run a model against each prompt and check it satisfies every expectation.", + "description": "Acceptance evals for the API-hygiene, effect-correctness, cancellation, and state-boundary behavior introduced in 4.1.x, plus Paging 3 in Compose list/load-state behavior added in 4.2.0 (cases 10-11), plus cross-phase back-write detection and the false-lead scoring guard added in 4.3.0 (cases 12-13, audit-only — compose-agent has no cross-phase / false-lead guidance yet). These are the audit/review acceptance criteria the skill must preserve; the earlier cases also back compose-agent review behavior. Run a model against each prompt and check it satisfies every expectation.", "evals": [ { "id": 0, @@ -147,6 +147,45 @@ "Recommend gating full-screen error on `itemCount == 0` and surfacing refresh errors that arrive with content as a snackbar / inline retry.", "Credit the correct stable `itemKey` already present instead of re-flagging it." ] + }, + { + "id": 12, + "prompt": "Audit this Compose layout for recomposition cost:\n@Composable fun PriceRow(label: String, price: String) {\n var labelWidth by remember { mutableIntStateOf(0) }\n Row {\n Text(label, Modifier.onSizeChanged { labelWidth = it.width })\n Spacer(Modifier.width(with(LocalDensity.current) { labelWidth.toDp() }))\n Text(price)\n }\n}", + "expected_output": "An audit flagging the cross-phase back-write: onSizeChanged (layout) writes labelWidth, which the Spacer reads in composition, forcing measure -> write -> recompose. Recommends consuming the measured width in the layout/draw phase (Modifier.layout {} / a custom Layout / SubcomposeLayout) instead of round-tripping through composition-read state.", + "files": [], + "expectations": [ + "Flag `onSizeChanged` writing `labelWidth` that is read in composition (by the `Spacer`) as a cross-phase (axis 3) back-write, not a same-body backwards write.", + "Identify both sides: the layout-phase writer (`onSizeChanged`) and the composition-phase reader (`labelWidth.toDp()` in the `Spacer`).", + "Recommend keeping the measured value in layout/draw — `Modifier.layout { }`, a custom `Layout`, or `SubcomposeLayout` — rather than a composition-read state hop. Do not accept `BoxWithConstraints`, which exposes parent constraints, not a sibling's measured width.", + "Do not misclassify it as a stability / unstable-parameter problem.", + "Treat it as Critical if the row is a reused lazy item or the size changes per frame." + ] + }, + { + "id": 13, + "prompt": "This screen targets Kotlin 2.1 / Compose Compiler with Strong Skipping on by default. A teammate says they 'optimized recomposition' with the two changes below. Audit whether these are real wins and whether to keep them:\n@Composable fun Rows(items: List, onOpen: (Row) -> Unit) {\n val stableOnOpen = remember(onOpen) { onOpen }\n LazyColumn { itemsIndexed(items) { index, row ->\n val isFirst = remember(index) { index == 0 }\n RowItem(row, isFirst, onClick = { stableOnOpen(row) })\n } }\n}", + "expected_output": "An audit that does not credit either change as a recomposition win under Strong Skipping: SSM already auto-memoizes lambdas so wrapping onOpen in remember is a no-op, and remember(index) around the pure `index == 0` saves no work. Neither should appear in Prioritized Fixes; any real advice points at runtime recomposition counts / compiler reports.", + "files": [], + "expectations": [ + "Do not credit `remember(onOpen) { onOpen }` as a stabilization win — under Strong Skipping lambdas are auto-memoized, so it is a no-op.", + "Do not credit `remember(index) { index == 0 }` — it wraps a pure, cheap expression keyed on its own input and saves nothing.", + "Do not list either pattern in Prioritized Fixes, and do not deduct for removing them.", + "If recommending anything, cite runtime recomposition counts or compiler reports as proof rather than the presence of the pattern.", + "Note that the `remember`-the-callback advice only applies with Strong Skipping OFF or on `@NonSkippableComposable` / `@DontMemoize` paths." + ] + }, + { + "id": 14, + "prompt": "Audit this overlay for recomposition cost:\n@Composable fun Overlay(rows: List) {\n val heights = remember { mutableStateMapOf() }\n rows.forEach { heights[it.id] = it.baseHeight }\n Column {\n rows.forEach { row ->\n Box(Modifier.height((heights[row.id] ?: 0).dp)) { RowContent(row) }\n }\n }\n}", + "expected_output": "An audit flagging the mutableStateMapOf mutated in the composable body (heights[it.id] = ...) that the same composition then reads, as composition-phase self-invalidation — scored once, not double-counted with the same-body backwards-write rule, and explicitly not a cross-phase (axis 3) back-write. Recommends deriving from params (remember(rows) { ... }) or moving the mutation into a state holder, rather than mutating snapshot state during composition.", + "files": [], + "expectations": [ + "Flag the `mutableStateMapOf` mutation (`heights[it.id] = ...`) in the composable body that the same composition reads as composition-phase self-invalidation.", + "Do NOT label it a cross-phase (axis 3) back-write — both the write and the read happen in composition.", + "Score it once — do not additionally deduct under the same-body backwards-write rule for the same code.", + "Recommend deriving the map from params (`remember(rows) { ... }`) or moving the mutation to a state holder / event, not mutating snapshot state during composition.", + "Do not flag this as an unstable-parameter / stability problem." + ] } ] } diff --git a/skills/jetpack-compose-audit/references/canonical-sources.md b/skills/jetpack-compose-audit/references/canonical-sources.md index b6e7675..3dc3579 100644 --- a/skills/jetpack-compose-audit/references/canonical-sources.md +++ b/skills/jetpack-compose-audit/references/canonical-sources.md @@ -32,7 +32,7 @@ These ground: - `derivedStateOf` - deferred state reads - lambda modifiers -- backwards writes +- backwards writes, cross-phase back-writes (layout → composition), snapshot-collection self-invalidation - stability annotations (`@Stable`, `@Immutable`), `kotlinx.collections.immutable`, `compose_compiler_config.conf` - Strong Skipping Mode (default since Kotlin 2.0.20), `@NonSkippableComposable`, `@DontMemoize` - Compose Compiler reports / metrics as the primary diagnostic for skippability and stability diff --git a/skills/jetpack-compose-audit/references/diagnostics.md b/skills/jetpack-compose-audit/references/diagnostics.md index dbd3799..8081578 100644 --- a/skills/jetpack-compose-audit/references/diagnostics.md +++ b/skills/jetpack-compose-audit/references/diagnostics.md @@ -190,5 +190,6 @@ When you arrive at a Compose repo, run these in order before scoring: 4. `rg -n 'isMinifyEnabled' -g '*.gradle*'` — release hygiene. 5. Run Step 4 of SKILL.md — the init script generates compiler reports automatically. If the build fails, read any existing `composeCompiler { reportsDestination ... }` output the project already produces; otherwise note the fallback in the report. 6. `rg -l 'baselineProfile|ProfileInstaller' -g '*.gradle*' -g '*.kt'` — baseline-profile presence. +7. `rg -n 'onSizeChanged|onGloballyPositioned|onPlaced' -g '*.kt'` and `rg -n 'mutableStateListOf|mutableStateMapOf|toMutableStateList|toMutableStateMap|SnapshotStateList|SnapshotStateMap' -g '*.kt'` — cross-phase back-write candidates (layout callback writing composition-read state) and snapshot-collection self-invalidation candidates. Read each hit before scoring — scope any mutation search to just these files, not the whole repo; see the cross-phase / self-invalidation heuristic in the search playbook. -These six checks tell you what kind of evidence is available before any rubric-level reading. +These seven checks tell you what kind of evidence is available before any rubric-level reading. diff --git a/skills/jetpack-compose-audit/references/scoring.md b/skills/jetpack-compose-audit/references/scoring.md index d63d414..d5362be 100644 --- a/skills/jetpack-compose-audit/references/scoring.md +++ b/skills/jetpack-compose-audit/references/scoring.md @@ -77,6 +77,7 @@ Reward: - `derivedStateOf` used for state that changes faster than its observable output (e.g. scroll position → "show button" boolean) → [docs](https://developer.android.com/develop/ui/compose/side-effects) - deferred reads via lambda modifiers (`Modifier.offset { … }`, `Modifier.graphicsLayer { … }`, `Modifier.drawBehind { … }`) → [docs](https://developer.android.com/develop/ui/compose/performance/bestpractices), [phases](https://developer.android.com/develop/ui/compose/performance/phases) - absence of backwards writes (writing to state that has already been read in the same composition) → [docs](https://developer.android.com/develop/ui/compose/performance/bestpractices) +- absence of cross-phase back-writes — measured size/position from a layout callback consumed in layout/draw (`Modifier.layout { }` / `Modifier.drawBehind { }`) rather than round-tripped through composition-read state → [phases](https://developer.android.com/develop/ui/compose/performance/phases) - stability hygiene: `@Stable` / `@Immutable` on data classes used as composable params → [docs](https://developer.android.com/develop/ui/compose/performance/stability) - `kotlinx.collections.immutable` (`ImmutableList`, `PersistentList`) for collection params — earns a skip when Strong Skipping is off, and under Strong Skipping still pays off via stable `equals()` / `hashCode()` (making the instance-equality check cheap and correct) and structural sharing (no per-recomposition `List.copy()`). Reward independently of the compiler track → [stability](https://developer.android.com/develop/ui/compose/performance/stability), [fix](https://developer.android.com/develop/ui/compose/performance/stability/fix) - `compose_compiler_config.conf` used to mark third-party types stable → [fix](https://developer.android.com/develop/ui/compose/performance/stability/fix) @@ -100,10 +101,12 @@ Deduct for: - reading rapidly changing state too high in the tree → [phases](https://developer.android.com/develop/ui/compose/performance/phases), [bestpractices](https://developer.android.com/develop/ui/compose/performance/bestpractices) - frequent-state values passed to non-lambda modifiers when a layout/draw-phase alternative exists → [bestpractices](https://developer.android.com/develop/ui/compose/performance/bestpractices) - backwards writes — writing to state already read in the same composition body → [bestpractices](https://developer.android.com/develop/ui/compose/performance/bestpractices) +- cross-phase back-writes (axis 3) — a **layout-phase** callback (`onSizeChanged` / `onGloballyPositioned` / `onPlaced`) writing state that a sibling **reads in composition** (measure → write → recompose loop). Deduct when a composition-phase read of the written state exists; promote to a Critical Finding when the loop is per-frame or spans reused lazy items → [phases](https://developer.android.com/develop/ui/compose/performance/phases), [bestpractices](https://developer.android.com/develop/ui/compose/performance/bestpractices) +- composition-phase self-invalidation — a `SnapshotStateMap` / `SnapshotStateList` mutated (`putAll` / `put` / `add` / `addAll` / `remove` / `clear` / `[k] =` / `+=` / `-=`) inside a `@Composable` body that also reads it. This overlaps the same-body backwards-write rule above — **score it once** (prefer the backwards-write rule for read→write; use this for the mutate-then-read / self-feeding shape). Not a cross-phase issue: the write and read are both in composition → [bestpractices](https://developer.android.com/develop/ui/compose/performance/bestpractices) - repeated broad recomposition smells across screens/components → [stability](https://developer.android.com/develop/ui/compose/performance/stability) - raw `List`/`Map`/`Set` parameters on widely reused composables when the rest of the codebase has the immutable-collections dependency available — deduct when Strong Skipping is OFF (unstable params block skipping outright), or when Strong Skipping is ON but the collection is rebuilt per recomposition in source (e.g. `listOf(a, b)` / `mapOf(...)` in a composable body, a getter that allocates, or `.toList()` / `.filter { }` on every call). Under Strong Skipping without observable churn, do not deduct → [stability](https://developer.android.com/develop/ui/compose/performance/stability) - unstable composable params whose `equals()` is expensive, allocating, or semantically broken — for example, a plain `class Foo(...)` with identity equality passed to a reusable composable, a `data class` wrapping a large collection (deep `equals` on every recomposition), or a `data class` with mutable fields (stale skip results). Under Strong Skipping, every recomposition runs `equals()` on each unstable param to decide whether to skip; expensive equality can make "skipping" as costly as recomposing, and broken equality makes skipping silently wrong → [strong skipping](https://developer.android.com/develop/ui/compose/performance/stability/strongskipping), [stability](https://developer.android.com/develop/ui/compose/performance/stability) -- instance-recreation churn in source: `listOf(...)` / `mapOf(...)` / `setOf(...)` / anonymous object or lambda literals / `MyParams(...)` allocated inside a hot composable body and passed as a param. The value is `!=` to the prior recomposition's value by default, so both vanilla skipping and Strong Skipping's `==` gate fail, forcing the callee to re-run body-and-children. Hoist into `remember(...)`, a state holder, or a `@Composable` caller further up → [stability](https://developer.android.com/develop/ui/compose/performance/stability), [strong skipping](https://developer.android.com/develop/ui/compose/performance/stability/strongskipping) +- instance-recreation churn in source: `listOf(...)` / `mapOf(...)` / `setOf(...)` / anonymous `object` literals / `MyParams(...)` allocated inside a hot composable body and passed as a param. The value is `!=` to the prior recomposition's value by default, so both vanilla skipping and Strong Skipping's `==` gate fail, forcing the callee to re-run body-and-children. Hoist into `remember(...)`, a state holder, or a `@Composable` caller further up. **Bare lambda literals are exempt under Strong Skipping** — the compiler auto-memoizes lambdas passed to composables, so do not treat a plain `onClick = { ... }` as churn or recommend wrapping it in `remember` (that is a false lead — see the guard below); a lambda churns only SSM-**off**, on a `@NonSkippableComposable` / `@DontMemoize` path, or when it captures an unstable value → [stability](https://developer.android.com/develop/ui/compose/performance/stability), [strong skipping](https://developer.android.com/develop/ui/compose/performance/stability/strongskipping) - `mutableStateOf` where the typed factory exists (autoboxing) → [state](https://developer.android.com/develop/ui/compose/state) - `derivedStateOf { ... }` whose block does not actually read any `State` object (meaning it will never invalidate, and the overhead of `derivedStateOf` is wasted) → [side-effects](https://developer.android.com/develop/ui/compose/side-effects) - `@NonSkippableComposable` / `@DontMemoize` opt-outs without a justifying comment → [strong skipping](https://developer.android.com/develop/ui/compose/performance/stability/strongskipping) @@ -170,7 +173,7 @@ Under Strong Skipping, all restartable composables become skippable and lambdas Under Strong Skipping the more informative evidence is no longer "how many unstable classes are there" but: -- **Instance-recreation churn in source** — `listOf(a, b)`, `mapOf(...)`, `{ ... }` object / lambda literals, or `MyParams(a, b)` allocated inside a composable body and passed as a param. The SSM `==` check fails on every recomposition and the callee re-runs regardless of skippability. +- **Instance-recreation churn in source** — `listOf(a, b)`, `mapOf(...)`, anonymous `object` literals, or `MyParams(a, b)` allocated inside a composable body and passed as a param. The SSM `==` check fails on every recomposition and the callee re-runs regardless of skippability. (Plain **lambda** literals are auto-memoized by SSM — exclude them here; a lambda only churns SSM-off, on an opt-out path, or when it captures an unstable value.) - **`equals()` cost and correctness on unstable params** — SSM calls `equals()` on each unstable param every recomposition. Expensive equality (`data class` wrapping a large collection, custom `equals` that walks a graph) can make skipping as costly as recomposing; broken equality (identity equality on a plain `class`, mutable fields inside a `data class`) makes skipping silently wrong. - **Runtime recomposition counts** (Layout Inspector, `Recomposer` debug output, Compose UI tests with `Modifier.testTag` + recomposition counters). When available these beat `skippable%` as the primary signal under SSM. - **`@NonSkippableComposable` / `@DontMemoize` opt-outs** — effectively the only way to see a "not skippable" named composable under SSM; each instance should carry a justifying comment and appear on hot paths only with cause. @@ -179,6 +182,22 @@ When compiler reports are **not** available (Step 4 failed, `Compiler diagnostic If a non-trivial subset of modules failed to build (partial reports), state which modules contributed and treat `skippable%` as a floor estimate rather than a ground truth. +#### Do Not Credit — False Leads + +Changes that *look* like recomposition fixes but do not reduce recomposition count. **Never reward them, and never emit them in `Prioritized Fixes`.** If the codebase already applies one, treat it as neutral (no credit); if a finding's suggested fix would be one of these, discard the finding or replace the fix with the real one. When code applies a false lead *in place of* the real fix, the underlying smell still stands and is scored on its own merits. + +| Looks like a fix | Why it does nothing | Real lever | +|---|---|---| +| `remember(index) { isFirstRow(index) }` (or any `remember(k)` wrapping a pure, cheap function of its own key) | Same inputs each call; the memo saves no work and adds bookkeeping — no skipping benefit | Inline the expression; only `remember` genuinely expensive work keyed on real inputs | +| Identity/instance cache for a read-only derived map (`remember { }` returning a cached map to preserve reference equality) | Can serve stale overlays; `remember(keys)` on the actual inputs is enough and correct | `remember(keys) { derive() }` — key on the inputs, not on identity | +| `mutableIntStateOf` + a layout modifier applied to **both** the measured row and its sibling | The sibling still reads the size **in composition** unless it is measure-only; boxing was never the problem | Make the sibling read measured size in layout/draw (`Modifier.layout { }`), or hoist the shared value out of the round-trip | +| Forcing `assertRecompositionCount(Exactly(1))` on **both** rows in a focus-move / selection test | One row often *correctly* recomposes 0 times; the assertion encodes a wrong expectation, not a fix | Assert the count each row should actually have; investigate only rows that exceed it | +| Manually `remember`-ing callbacks **under Strong Skipping** (Kotlin 2.0.20+ default) to "stabilize" them | SSM auto-memoizes lambdas already; wrapping them in `remember` adds bookkeeping for no skipping benefit | Nothing — SSM has it covered. Only matters SSM-**off** (or when the lambda is on a `@NonSkippableComposable` / `@DontMemoize` path, or captures an unstable value): then hoist **and** stabilize, or pass a method reference | + +**Strong Skipping caveat for the last row:** the "fresh lambda defeats skipping" reasoning only holds when SSM is **off**, or the lambda sits on an opt-out path (`@NonSkippableComposable` / `@DontMemoize`), or it captures an unstable value. On the default SSM track with stable captures, manual `remember` on the callback is itself the no-op — do not recommend it, and do not deduct for its absence. Decide which compiler track applies (see the Strong Skipping Mode Check) before scoring this one. + +Route: the cross-phase and stability *detectors* live in the search playbook; this false-lead guard lives here in scoring and gates what those detectors are allowed to recommend. Before crediting any "we optimized recomposition here" claim, confirm it is not one of the above — prefer runtime recomposition counts or compiler reports as proof over the presence of the pattern. + ### State Management Reward: diff --git a/skills/jetpack-compose-audit/references/search-playbook.md b/skills/jetpack-compose-audit/references/search-playbook.md index b8a8c99..19b2fbc 100644 --- a/skills/jetpack-compose-audit/references/search-playbook.md +++ b/skills/jetpack-compose-audit/references/search-playbook.md @@ -340,6 +340,8 @@ Do not open a new score category for Nav3. If the project has no Nav2 code at al - scroll or animation state read high in the tree - fast-changing values passed to non-lambda modifiers when a layout/draw-phase alternative exists - backwards writes — *writing to state that has already been read in the same composition body* (this is the precise definition; reading after writing is fine) +- cross-phase back-writes — a **layout-phase write into state an earlier composition read**, re-invalidating it: a layout callback (`onSizeChanged`, `onGloballyPositioned`, `onPlaced`) writes a `mutableStateOf` / `mutableIntStateOf` that a **sibling reads in composition** → measure → write → recompose loop, often per-frame during resize/scroll. This is axis 3 proper (a later phase feeding an earlier one). See the heuristic below +- composition-phase self-invalidation — a **`SnapshotStateMap` / `SnapshotStateList` mutated inside a `@Composable` body** (`putAll`, `put`, `add`, `addAll`, `remove`, `clear`, `[k] =`, `+=`, `-=`) that the same composition also reads. This is *not* cross-phase — the write and read are both in composition; it is a close cousin of the same-body backwards-write rule above. Score it **once**: if the body reads the collection and *then* mutates it, the existing backwards-write rule already covers it — flag it here only when the mutation-then-read / self-feeding shape would slip past that rule's "already read" wording - `mutableStateOf` / `` / `` / `` — the typed factories avoid boxing - raw `List`/`Map`/`Set` parameters on widely reused composables when `kotlinx.collections.immutable` is already a dependency - `@NonSkippableComposable` / `@DontMemoize` without a justifying comment @@ -434,6 +436,20 @@ Positive signals to reward: - `AnimatedContent(targetState, label = "...")` when the call site needs custom enter/exit or size-aware transitions; `Crossfade` for standard fade-only swaps - reusable animated components exposing `animationSpec: AnimationSpec` when callers need timing control, and using meaningful labels on tooling-visible animations +### Cross-Phase Back-Write & Composition Self-Invalidation Heuristic + +Two related re-invalidation smells. Only shape 1 is truly cross-phase (axis 3); shape 2 stays within composition and overlaps the same-body backwards-write rule — keep them distinct so the report neither mislabels nor double-deducts. No single regex proves either; gather candidates, then read each call site. + +1. **Layout → composition writes (cross-phase, axis 3).** Layout runs after composition, so a layout-phase write into state read in composition re-runs composition. List the layout callbacks: `rg -n 'onSizeChanged|onGloballyPositioned|onPlaced' -g '*.kt'`. For each hit, check whether its lambda **writes** a state holder (`something = ...`, `state.value = ...`, `.intValue =`) rather than only reading, then confirm that written state is **read during composition** — by a sibling composable, a parent, or the same subtree (not only inside a deferred lambda modifier). It is legitimate only when the value is read exclusively in a later layout/draw pass (e.g. fed straight into `Modifier.layout { }` / `Modifier.drawBehind { }`); flag it when a composition-phase read exists. +2. **Snapshot-collection self-invalidation (composition phase, *not* cross-phase).** First find snapshot collections: `rg -n 'mutableStateListOf|mutableStateMapOf|toMutableStateList|toMutableStateMap|SnapshotStateList|SnapshotStateMap' -g '*.kt'` (the `to*` builders create the same types without naming the factory). Then, **only inside the files that hit above** (a global mutation search is pure noise — every `list.add(...)` in the repo matches), read each usage and flag a mutation — `putAll` / `put` / `add` / `addAll` / `remove` / `clear` / `[k] =` / `+=` / `-=` — that sits **directly in a `@Composable` body** (not in an event handler, `LaunchedEffect`, or state holder) while the same composition also reads the collection. Deduct **once**: prefer the same-body backwards-write rule when the body reads before it mutates; use this shape for the mutate-then-read / self-feeding case that the "already read" wording would miss. + +Severity: **Should-fix**, promoted to a **Critical Finding** when the loop is per-frame (resize/scroll/animation-driven) or spans reused lazy items. For shape 1, cite the writer's `file:line` and the reader's `file:line`. Docs: [phases](https://developer.android.com/develop/ui/compose/performance/phases), [bestpractices](https://developer.android.com/develop/ui/compose/performance/bestpractices). + +Positive signals to reward: + +- a measured size / position written from a layout callback is consumed only in `Modifier.layout { }` / `Modifier.offset { }` / `Modifier.drawBehind { }` (stays in layout/draw, never re-enters composition) +- `SubcomposeLayout` / `BoxWithConstraints` used where a child genuinely needs parent constraints, instead of round-tripping measured size through composition-read state + ### Strong Skipping Mode Check Confirm the project's compiler version: