From 206ff75f3ae91d851c312c5534e5b4f82d10ebb3 Mon Sep 17 00:00:00 2001 From: Ivan Morgillo Date: Mon, 6 Jul 2026 17:08:27 +0200 Subject: [PATCH 1/4] feat(audit): cross-phase back-write detection + false-lead guard (4.3.0) jetpack-compose-audit gains axis 3 of the recomposition problem, plus a scoring guard against no-op "fixes". Axes 1-2 (stability/skipping, deferred reads) were already covered by this suite. - search-playbook.md: Cross-Phase Back-Write Heuristic (layout callbacks onSizeChanged/onGloballyPositioned/onPlaced writing state read in composition; SnapshotStateMap/List mutated in a @Composable body) + red-flag bullet next to the existing same-body backwards-write rule. - scoring.md: matching Performance deduction (Critical when per-frame or across reused lazy items) + "Do Not Credit - False Leads" table so the auditor stops rewarding/suggesting remember(index) on pure fns, identity caches for derived maps, Exactly(1) on both rows, hoisting without stabilizing lambdas, etc. - diagnostics.md: Quick Triage Recipe step 7 to surface candidates up front. - Version: jetpack-compose-audit -> 4.3.0 (SKILL + both plugin.json). compose-agent unchanged at 4.2.1. Axis 3 and the false-lead cases adapted (reworded, re-cited against developer.android.com) from chrisbanes/skills compose-recomposition-performance (Apache-2.0). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_016ysskXsKpwEbs1n9CX1ScY --- CHANGELOG.md | 9 +++++++++ README.md | 11 ++++++++++- .../.claude-plugin/plugin.json | 2 +- .../.cursor-plugin/plugin.json | 2 +- skills/jetpack-compose-audit/SKILL.md | 2 +- .../references/diagnostics.md | 3 ++- .../jetpack-compose-audit/references/scoring.md | 15 +++++++++++++++ .../references/search-playbook.md | 15 +++++++++++++++ 8 files changed, 54 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d68a597..4973a3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ 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 (new detector).** The audit already deducted for same-body backwards writes; it now also catches the subtler cross-phase shape — a later phase writing snapshot state an earlier phase read. `search-playbook.md` §3 gains a **Cross-Phase Back-Write Heuristic** (layout callbacks `onSizeChanged` / `onGloballyPositioned` / `onPlaced` writing state a sibling reads in composition; `SnapshotStateMap` / `SnapshotStateList` mutated inside a `@Composable` body). `scoring.md` adds the matching Performance deduction (Critical when the loop is per-frame or spans reused lazy items). `diagnostics.md` Quick Triage Recipe gains step 7 to surface candidates up front. Findings cite writer and reader `file:line`. +- **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, hoisting without stabilizing lambda captures). 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`. + ### 4.2.2 — 2026-06-30 **`jetpack-compose-audit` only — Navigation 3 audit patterns.** diff --git a/README.md b/README.md index 1e0b675..7a7a419 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 (layout callbacks writing state read in composition; snapshot collections mutated in a composable body) plus a **False Leads** scoring guard so the auditor stops crediting no-op "recomposition fixes." Axis 3 of the recomposition problem, 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,15 @@ 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.** Beyond same-body backwards writes, the audit now flags a later phase writing snapshot state an earlier phase read: `onSizeChanged` / `onGloballyPositioned` / `onPlaced` (layout) writing state a sibling reads in composition, or `SnapshotStateMap` / `SnapshotStateList` mutated inside a `@Composable` body. New **Cross-Phase Back-Write 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`. +- **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`. + ### 4.2.1 — 2026-06-29 **`compose-agent` — Navigation 3 decision table.** 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..df4d50f 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: diff --git a/skills/jetpack-compose-audit/references/diagnostics.md b/skills/jetpack-compose-audit/references/diagnostics.md index dbd3799..4ab278b 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' -g '*.kt'` — cross-phase back-write candidates (layout callback writing composition-read state; snapshot collection mutated in a composable body). Read each hit before scoring; see the cross-phase back-write 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..48b0477 100644 --- a/skills/jetpack-compose-audit/references/scoring.md +++ b/skills/jetpack-compose-audit/references/scoring.md @@ -100,6 +100,7 @@ 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 — a later phase writing snapshot state an earlier phase already read: a layout-phase callback (`onSizeChanged` / `onGloballyPositioned` / `onPlaced`) writing state that a sibling reads in composition, or a `SnapshotStateMap` / `SnapshotStateList` mutated (`putAll` / `add` / `clear` / `[k] =`) inside a `@Composable` body that also reads it. Deduct when a composition-phase read of the written state exists (measure → write → recompose loop); 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) - 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) @@ -179,6 +180,20 @@ 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 | +| Hoisting state up without stabilizing the lambda captures passed back down | A fresh lambda instance every recomposition still defeats skipping on the child | Hoist **and** stabilize — `remember` the callbacks, or pass method references / stable event objects | + +Route: these mirror the search-playbook cross-phase and stability heuristics. 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..3e6c13a 100644 --- a/skills/jetpack-compose-audit/references/search-playbook.md +++ b/skills/jetpack-compose-audit/references/search-playbook.md @@ -340,6 +340,7 @@ 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 **later phase writes snapshot state that an earlier phase already read**, re-invalidating it. Two shapes to verify: (1) a **layout-phase** callback (`onSizeChanged`, `onGloballyPositioned`, `onPlaced`) writes a `mutableStateOf` / `mutableIntStateOf` that a **sibling reads in composition** → measure → write → recompose loop, often on every frame during resize/scroll; (2) a **`SnapshotStateMap` / `SnapshotStateList` mutated inside a `@Composable` body** (`putAll`, `add`, `remove`, `clear`, `[k] =`) while the same composition reads it → the write invalidates the composition that produced it. See the cross-phase back-write heuristic below - `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 +435,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 Heuristic + +A later phase writing state an earlier phase already read forces re-invalidation — the subtle cousin of same-body backwards writes. Layout runs after composition and draw runs after layout, so a write from layout/draw into state read in composition re-runs composition. No single regex proves it; gather candidates, then read each call site. + +1. **Layout → composition writes.** 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). That is a layout→composition back-write. 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 mutation during composition.** Find snapshot collections: `rg -n 'mutableStateListOf|mutableStateMapOf|SnapshotStateList|SnapshotStateMap' -g '*.kt'`. Then look for mutations — `rg -n '\.(putAll|put|add|addAll|remove|clear)\(|\]\s*=' -g '*.kt'` — that sit **directly in a `@Composable` body** (not in an event handler, `LaunchedEffect`, or state holder). A collection mutated and read in the same composition re-invalidates it every pass. + +Severity: **Should-fix**, promoted to **Blocker** when the loop is per-frame (resize/scroll/animation-driven) or spans reused lazy items. Cite the writer's `file:line` and the reader's `file:line` in the finding. 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: From 4e5cefce18e60a49b0f2475e78bbcb68d492269e Mon Sep 17 00:00:00 2001 From: Ivan Morgillo Date: Mon, 6 Jul 2026 17:20:28 +0200 Subject: [PATCH 2/4] review: fix taxonomy, SSM false-lead, regex scope + eval coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses round-1 cross-review (Codex, Cursor, Antigravity) — no blockers, these are the should-fixes: - Taxonomy: split axis-3 cross-phase (layout->composition) from composition-phase self-invalidation (snapshot-collection mutation in a @Composable body). The latter is NOT cross-phase; add a score-once note vs the existing same-body backwards-write rule to avoid double-deduction. (search-playbook, scoring, diagnostics, SKILL, canonical-sources) - SSM false-lead correctness: manually remember-ing callbacks is itself the no-op under Strong Skipping (auto-memoized); the "fresh lambda defeats skipping" lever only holds SSM-off / opt-out paths / unstable captures. Reworded the table row + added a compiler-track caveat. - Agent context blow-up: scope the snapshot-mutation search to the files that hit the collection search, not the whole repo. Added += / -= operators. - diagnostics step 7 regex aligned with the playbook (SnapshotStateList/Map). - SKILL.md Performance focus + canonical-sources list now name the new shapes. - Changelog: note the manifest jump 4.2.0 -> 4.3.0 (4.2.2 was docs-only). - evals.json: cases 12 (cross-phase back-write) and 13 (false-lead must not be credited/suggested under SSM) lock in the 4.3.0 behavior. - Softened the false-lead routing line: detectors live in the playbook, the guard lives in scoring. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_016ysskXsKpwEbs1n9CX1ScY --- CHANGELOG.md | 2 +- skills/jetpack-compose-audit/SKILL.md | 2 +- skills/jetpack-compose-audit/evals/evals.json | 28 ++++++++++++++++++- .../references/canonical-sources.md | 2 +- .../references/diagnostics.md | 2 +- .../references/scoring.md | 9 ++++-- .../references/search-playbook.md | 13 +++++---- 7 files changed, 44 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4973a3c..47a7754 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ Full release history for the Compose Skill Suite. The newest release is summaris - **Cross-phase back-writes (new detector).** The audit already deducted for same-body backwards writes; it now also catches the subtler cross-phase shape — a later phase writing snapshot state an earlier phase read. `search-playbook.md` §3 gains a **Cross-Phase Back-Write Heuristic** (layout callbacks `onSizeChanged` / `onGloballyPositioned` / `onPlaced` writing state a sibling reads in composition; `SnapshotStateMap` / `SnapshotStateList` mutated inside a `@Composable` body). `scoring.md` adds the matching Performance deduction (Critical when the loop is per-frame or spans reused lazy items). `diagnostics.md` Quick Triage Recipe gains step 7 to surface candidates up front. Findings cite writer and reader `file:line`. - **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, hoisting without stabilizing lambda captures). 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`. +- **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 diff --git a/skills/jetpack-compose-audit/SKILL.md b/skills/jetpack-compose-audit/SKILL.md index df4d50f..2180a95 100644 --- a/skills/jetpack-compose-audit/SKILL.md +++ b/skills/jetpack-compose-audit/SKILL.md @@ -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..fc7b212 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). 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.", "evals": [ { "id": 0, @@ -147,6 +147,32 @@ "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` / `BoxWithConstraints` — rather than a composition-read state hop.", + "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." + ] } ] } 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 4ab278b..f38a709 100644 --- a/skills/jetpack-compose-audit/references/diagnostics.md +++ b/skills/jetpack-compose-audit/references/diagnostics.md @@ -190,6 +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' -g '*.kt'` — cross-phase back-write candidates (layout callback writing composition-read state; snapshot collection mutated in a composable body). Read each hit before scoring; see the cross-phase back-write heuristic in the search playbook. +7. `rg -n 'onSizeChanged|onGloballyPositioned|onPlaced' -g '*.kt'` and `rg -n 'mutableStateListOf|mutableStateMapOf|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 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 48b0477..182be05 100644 --- a/skills/jetpack-compose-audit/references/scoring.md +++ b/skills/jetpack-compose-audit/references/scoring.md @@ -100,7 +100,8 @@ 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 — a later phase writing snapshot state an earlier phase already read: a layout-phase callback (`onSizeChanged` / `onGloballyPositioned` / `onPlaced`) writing state that a sibling reads in composition, or a `SnapshotStateMap` / `SnapshotStateList` mutated (`putAll` / `add` / `clear` / `[k] =`) inside a `@Composable` body that also reads it. Deduct when a composition-phase read of the written state exists (measure → write → recompose loop); 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) +- 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` / `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) @@ -190,9 +191,11 @@ Changes that *look* like recomposition fixes but do not reduce recomposition cou | 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 | -| Hoisting state up without stabilizing the lambda captures passed back down | A fresh lambda instance every recomposition still defeats skipping on the child | Hoist **and** stabilize — `remember` the callbacks, or pass method references / stable event objects | +| 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 | -Route: these mirror the search-playbook cross-phase and stability heuristics. 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. +**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 diff --git a/skills/jetpack-compose-audit/references/search-playbook.md b/skills/jetpack-compose-audit/references/search-playbook.md index 3e6c13a..2b31aac 100644 --- a/skills/jetpack-compose-audit/references/search-playbook.md +++ b/skills/jetpack-compose-audit/references/search-playbook.md @@ -340,7 +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 **later phase writes snapshot state that an earlier phase already read**, re-invalidating it. Two shapes to verify: (1) a **layout-phase** callback (`onSizeChanged`, `onGloballyPositioned`, `onPlaced`) writes a `mutableStateOf` / `mutableIntStateOf` that a **sibling reads in composition** → measure → write → recompose loop, often on every frame during resize/scroll; (2) a **`SnapshotStateMap` / `SnapshotStateList` mutated inside a `@Composable` body** (`putAll`, `add`, `remove`, `clear`, `[k] =`) while the same composition reads it → the write invalidates the composition that produced it. See the cross-phase back-write heuristic below +- cross-phase back-writes — a **layout- or draw-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 @@ -435,14 +436,14 @@ 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 Heuristic +### Cross-Phase Back-Write & Composition Self-Invalidation Heuristic -A later phase writing state an earlier phase already read forces re-invalidation — the subtle cousin of same-body backwards writes. Layout runs after composition and draw runs after layout, so a write from layout/draw into state read in composition re-runs composition. No single regex proves it; gather candidates, then read each call site. +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.** 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). That is a layout→composition back-write. 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 mutation during composition.** Find snapshot collections: `rg -n 'mutableStateListOf|mutableStateMapOf|SnapshotStateList|SnapshotStateMap' -g '*.kt'`. Then look for mutations — `rg -n '\.(putAll|put|add|addAll|remove|clear)\(|\]\s*=' -g '*.kt'` — that sit **directly in a `@Composable` body** (not in an event handler, `LaunchedEffect`, or state holder). A collection mutated and read in the same composition re-invalidates it every pass. +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|SnapshotStateList|SnapshotStateMap' -g '*.kt'`. 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 **Blocker** when the loop is per-frame (resize/scroll/animation-driven) or spans reused lazy items. Cite the writer's `file:line` and the reader's `file:line` in the finding. Docs: [phases](https://developer.android.com/develop/ui/compose/performance/phases), [bestpractices](https://developer.android.com/develop/ui/compose/performance/bestpractices). +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: From bd6624c604a383e31183471ced35bd92e3ca9075 Mon Sep 17 00:00:00 2001 From: Ivan Morgillo Date: Mon, 6 Jul 2026 17:33:33 +0200 Subject: [PATCH 3/4] review r2: SSM lambda-churn consistency, eval fixes, copy tightening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 cross-review (Codex, Cursor) — no blockers: - SSM contradiction (Codex): the instance-recreation-churn rule no longer lists bare lambda literals as churn-to-remember — SSM auto-memoizes lambdas, so that would reintroduce the exact no-op advice the false-lead guard suppresses. Qualified both the Deduct bullet and the SSM-ON prose (lambdas churn only SSM-off / opt-out paths / unstable captures). - eval 12 (Codex, Cursor): dropped BoxWithConstraints from the accepted fix — it exposes parent constraints, not a sibling's measured width. Keep Modifier.layout / custom Layout / SubcomposeLayout. - addAll added to the scoring snapshot-mutation list (parity with the playbook). - red-flag narrowed from "layout- or draw-phase" to "layout-phase" to match the detectors (onSizeChanged/onGloballyPositioned/onPlaced). - Reward bullet added for absence of cross-phase back-writes (rubric symmetry). - evals description no longer claims cases 12-13 back compose-agent behavior (audit-only). - README/CHANGELOG: separated cross-phase (axis 3) from composition-phase self-invalidation; false-lead example now matches the table (manual remember under SSM), category table lists cross-phase back-writes. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_016ysskXsKpwEbs1n9CX1ScY --- CHANGELOG.md | 2 +- README.md | 7 ++++--- skills/jetpack-compose-audit/evals/evals.json | 4 ++-- skills/jetpack-compose-audit/references/scoring.md | 7 ++++--- skills/jetpack-compose-audit/references/search-playbook.md | 2 +- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47a7754..84ffc6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ Full release history for the Compose Skill Suite. The newest release is summaris **`jetpack-compose-audit` only — cross-phase back-write detection + false-lead guard.** - **Cross-phase back-writes (new detector).** The audit already deducted for same-body backwards writes; it now also catches the subtler cross-phase shape — a later phase writing snapshot state an earlier phase read. `search-playbook.md` §3 gains a **Cross-Phase Back-Write Heuristic** (layout callbacks `onSizeChanged` / `onGloballyPositioned` / `onPlaced` writing state a sibling reads in composition; `SnapshotStateMap` / `SnapshotStateList` mutated inside a `@Composable` body). `scoring.md` adds the matching Performance deduction (Critical when the loop is per-frame or spans reused lazy items). `diagnostics.md` Quick Triage Recipe gains step 7 to surface candidates up front. Findings cite writer and reader `file:line`. -- **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, hoisting without stabilizing lambda captures). 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. +- **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. diff --git a/README.md b/README.md index 7a7a419..8cff534 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ Claude Code plugin

-**`jetpack-compose-audit` 4.3.0 · 2026-07-06** — Cross-phase back-write detection (layout callbacks writing state read in composition; snapshot collections mutated in a composable body) plus a **False Leads** scoring guard so the auditor stops crediting no-op "recomposition fixes." Axis 3 of the recomposition problem, adapted from [`chrisbanes/skills`](https://github.com/chrisbanes/skills) (Apache-2.0). `compose-agent` stays at `4.2.1`. +**`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`. @@ -33,7 +33,8 @@ Authored and cross-reviewed with every frontier model — Claude Opus 4.8, GPT-5 **`jetpack-compose-audit` — cross-phase back-writes + false-lead guard.** -- **Cross-phase back-write detector.** Beyond same-body backwards writes, the audit now flags a later phase writing snapshot state an earlier phase read: `onSizeChanged` / `onGloballyPositioned` / `onPlaced` (layout) writing state a sibling reads in composition, or `SnapshotStateMap` / `SnapshotStateList` mutated inside a `@Composable` body. New **Cross-Phase Back-Write 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`. +- **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`. @@ -77,7 +78,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/skills/jetpack-compose-audit/evals/evals.json b/skills/jetpack-compose-audit/evals/evals.json index fc7b212..231c919 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), plus cross-phase back-write detection and the false-lead scoring guard added in 4.3.0 (cases 12-13). 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, @@ -156,7 +156,7 @@ "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` / `BoxWithConstraints` — rather than a composition-read state hop.", + "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." ] diff --git a/skills/jetpack-compose-audit/references/scoring.md b/skills/jetpack-compose-audit/references/scoring.md index 182be05..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) @@ -101,11 +102,11 @@ Deduct for: - 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` / `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) +- 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) @@ -172,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. diff --git a/skills/jetpack-compose-audit/references/search-playbook.md b/skills/jetpack-compose-audit/references/search-playbook.md index 2b31aac..a9b44a6 100644 --- a/skills/jetpack-compose-audit/references/search-playbook.md +++ b/skills/jetpack-compose-audit/references/search-playbook.md @@ -340,7 +340,7 @@ 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- or draw-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 +- 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 From fe125728ae5d4a5a500976180aea9e5351aed633 Mon Sep 17 00:00:00 2001 From: Ivan Morgillo Date: Mon, 6 Jul 2026 17:45:28 +0200 Subject: [PATCH 4/4] review r3: toMutableState* detection, changelog split, self-invalidation eval, 4.3.0 release notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 cross-review (Codex, Cursor) — no blockers: - Detector coverage (Codex): snapshot search now includes toMutableStateList() / toMutableStateMap() builders (create the types without naming the factory) in both the playbook heuristic and diagnostics step 7. - CHANGELOG split (Codex, Cursor): cross-phase (axis 3) and composition-phase self-invalidation are now separate bullets; the "cite writer+reader file:line" claim is scoped to the cross-phase shape only. - CHANGELOG 4.2.2 reconciled: the 4.2.2 entry now says content-only, matching the 4.3.0 note about the manifest jump. - eval 14 (Cursor): acceptance case for snapshot-collection self-invalidation in a @Composable body — must not be labeled cross-phase, scored once, not a stability finding. - docs/release-notes-4.3.0.md added for parity with prior releases; linked from README What's new. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_016ysskXsKpwEbs1n9CX1ScY --- CHANGELOG.md | 5 ++- README.md | 2 + docs/release-notes-4.3.0.md | 41 +++++++++++++++++++ skills/jetpack-compose-audit/evals/evals.json | 13 ++++++ .../references/diagnostics.md | 2 +- .../references/search-playbook.md | 2 +- 6 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 docs/release-notes-4.3.0.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 84ffc6a..87f8e2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ Full release history for the Compose Skill Suite. The newest release is summaris **`jetpack-compose-audit` only — cross-phase back-write detection + false-lead guard.** -- **Cross-phase back-writes (new detector).** The audit already deducted for same-body backwards writes; it now also catches the subtler cross-phase shape — a later phase writing snapshot state an earlier phase read. `search-playbook.md` §3 gains a **Cross-Phase Back-Write Heuristic** (layout callbacks `onSizeChanged` / `onGloballyPositioned` / `onPlaced` writing state a sibling reads in composition; `SnapshotStateMap` / `SnapshotStateList` mutated inside a `@Composable` body). `scoring.md` adds the matching Performance deduction (Critical when the loop is per-frame or spans reused lazy items). `diagnostics.md` Quick Triage Recipe gains step 7 to surface candidates up front. Findings cite writer and reader `file:line`. +- **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. @@ -19,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 8cff534..85f65fc 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,8 @@ Authored and cross-reviewed with every frontier model — Claude Opus 4.8, GPT-5 - **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.** 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/evals/evals.json b/skills/jetpack-compose-audit/evals/evals.json index 231c919..2286ad9 100644 --- a/skills/jetpack-compose-audit/evals/evals.json +++ b/skills/jetpack-compose-audit/evals/evals.json @@ -173,6 +173,19 @@ "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/diagnostics.md b/skills/jetpack-compose-audit/references/diagnostics.md index f38a709..8081578 100644 --- a/skills/jetpack-compose-audit/references/diagnostics.md +++ b/skills/jetpack-compose-audit/references/diagnostics.md @@ -190,6 +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|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. +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 seven checks tell you what kind of evidence is available before any rubric-level reading. diff --git a/skills/jetpack-compose-audit/references/search-playbook.md b/skills/jetpack-compose-audit/references/search-playbook.md index a9b44a6..19b2fbc 100644 --- a/skills/jetpack-compose-audit/references/search-playbook.md +++ b/skills/jetpack-compose-audit/references/search-playbook.md @@ -441,7 +441,7 @@ Positive signals to reward: 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|SnapshotStateList|SnapshotStateMap' -g '*.kt'`. 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. +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).