Skip to content

feat(audit): cross-phase back-write detection + false-lead guard (4.3.0)#10

Merged
hamen merged 4 commits into
mainfrom
feat/cross-phase-backwrite-false-leads
Jul 6, 2026
Merged

feat(audit): cross-phase back-write detection + false-lead guard (4.3.0)#10
hamen merged 4 commits into
mainfrom
feat/cross-phase-backwrite-false-leads

Conversation

@hamen

@hamen hamen commented Jul 6, 2026

Copy link
Copy Markdown
Owner

What

Adds axis 3 of the recomposition problem to jetpack-compose-audit, a related composition-phase self-invalidation check, and a scoring guard against no-op "fixes". Prompted by chrisbanes/skills compose-recomposition-performance (Apache-2.0) — this suite already covered axes 1–2 (stability/skipping, deferred reads), so only the genuinely-missing parts are imported, reworded, and re-cited against developer.android.com.

Why

The audit already deducted for same-body backwards writes but was blind to the subtler cross-phase shape (a layout-phase write into state composition read) and had no guard against crediting plausible-but-useless recomposition "fixes".

Changes

File Change
search-playbook.md Cross-phase / self-invalidation heuristic + red-flag bullets (deduped vs same-body rule)
scoring.md Cross-phase deduction (Critical when per-frame / reused lazy items), self-invalidation deduction (score-once), Reward bullet, Do Not Credit — False Leads table, SSM lambda-churn exemption
diagnostics.md Quick Triage step 7, scoped to candidate files
evals/evals.json Cases 12 (cross-phase), 13 (false leads under SSM), 14 (self-invalidation)
versions jetpack-compose-audit4.3.0; compose-agent unchanged at 4.2.1
CHANGELOG.md, README.md, docs/release-notes-4.3.0.md 4.3.0 notes + attribution

Detectors

  • Cross-phase back-write (axis 3): onSizeChanged / onGloballyPositioned / onPlaced (layout) writing state a sibling reads in composition. Cites writer + reader file:line.
  • Composition-phase self-invalidation (related, not cross-phase): SnapshotStateMap / SnapshotStateList (incl. toMutableState* builders) mutated in a @Composable body. Deduped against the same-body rule.
  • False leads (guard): remember(index) on pure fns, identity caches for derived maps, layout modifiers on both rows, Exactly(1) on both rows, and — under Strong Skipping — manually remember-ing an already auto-memoized callback. Never rewarded, never emitted in Prioritized Fixes.

Vibe

Stays an auditor, not a tutorial: point it at your repo → it names the exact file:line where you're back-writing across phases and losing frames.

Review

Three rounds of cross-review (Codex, Cursor, Antigravity) — no blockers; all should-fixes addressed (taxonomy split, SSM lambda correctness, BoxWithConstraints eval fix, toMutableState* coverage, dedup guard, eval coverage). bin/ci ✅. Attribution to chrisbanes/skills (Apache-2.0).

🤖 Generated with Claude Code

https://claude.ai/code/session_016ysskXsKpwEbs1n9CX1ScY

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ysskXsKpwEbs1n9CX1ScY
@hamen

hamen commented Jul 6, 2026

Copy link
Copy Markdown
Owner Author
🟠 Antigravity review (automated cross-review)

Overall, this is a solid update to catch subtle performance bugs and prevent false-positive scoring. However, there are a few issues to address, primarily around regex usage for the AI agent.

Should-fix / Blocker

  • Agent Context Explosion: Instructing the AI in search-playbook.md to run rg -n '\.(putAll|put|add|addAll|remove|clear)\(|\]\s*=' -g '*.kt' globally will return massive noise (matching every standard .add() and map assignment across the entire project). This is very likely to blow up the agent's context window. The playbook should explicitly instruct the agent to constrain this second search only to the specific files found in step 1, or to simply inspect the usages of the found collections.
  • Inaccurate Terminology: Mutating a SnapshotStateList or SnapshotStateMap directly inside a @Composable body happens entirely within the Composition phase. This is technically a "same-body backward write," not a "cross-phase" back-write (which strictly involves layout/draw writing to composition). Grouping them together makes the definition factually incorrect.

Nit

  • Inconsistent Search Targets: Step 7 in diagnostics.md only searches for mutableStateListOf|mutableStateMapOf, but search-playbook.md correctly also includes SnapshotStateList|SnapshotStateMap. These commands should match.
  • Missing Mutation Operators: The regex \.(putAll|put|add|addAll|remove|clear)\(|\]\s*= misses standard Kotlin assignment operators like += and -= (e.g., list += item), which are extremely common ways to mutate snapshot lists.

via pr-review-relay · author: claude

hamen and others added 2 commits July 6, 2026 17:20
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ysskXsKpwEbs1n9CX1ScY
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ysskXsKpwEbs1n9CX1ScY
@hamen

hamen commented Jul 6, 2026

Copy link
Copy Markdown
Owner Author
🟢 Codex review (automated cross-review)

Blocker
None.

Should-fix

  • diagnostics.md / search-playbook.md: the snapshot-collection detector misses toMutableStateList() / toMutableStateMap(). Those create SnapshotStateList / SnapshotStateMap without spelling the factory or type, and because the playbook scopes mutation inspection only to files found by this query, these self-invalidation cases become invisible.

Nit

  • CHANGELOG.md: the 4.3.0 cross-phase bullet still folds SnapshotStateMap / SnapshotStateList composable-body mutation into the cross-phase detector, while the actual rubric correctly says that shape is composition-phase, not cross-phase.
  • CHANGELOG.md: this says 4.2.2 never bumped manifests, but CHANGELOG.md still says jetpack-compose-audit moved to 4.2.2. One of those release-history statements should be corrected.

bin/ci passes.

via pr-review-relay · author: claude

@hamen

hamen commented Jul 6, 2026

Copy link
Copy Markdown
Owner Author
🔵 Cursor review (automated cross-review)

PR #10 review — feat(audit): cross-phase back-write detection + false-lead guard (4.3.0)

Docs-only release; the core guidance in search-playbook.md, scoring.md, diagnostics.md, and evals 12–13 is technically sound and internally consistent. No security issues. Nothing here is a merge blocker.


Blocker

None.


Should-fix

  1. CHANGELOG conflates two detectors. The 4.3.0 bullet bundles SnapshotStateMap / SnapshotStateList mutation into “cross-phase back-writes,” but the skill correctly treats that as composition-phase self-invalidation (same phase, deduped against backwards-write). README already splits them; CHANGELOG should match.

  2. CHANGELOG overclaims file:line attribution. “Findings cite writer and reader file:line” fits layout→composition (shape 1) only. Self-invalidation is usually one composable body, not a writer/reader pair.

  3. PR description vs. shipped false-lead table. The PR body lists “hoisting without stabilizing lambda captures,” but scoring.md replaces that with the more precise “Manually remember-ing callbacks under Strong Skipping” row (with SSM-off caveats). Align PR/CHANGELOG wording with the table.

  4. No eval for self-invalidation (shape 2). Evals 12–13 cover cross-phase and false leads; there is no acceptance case for snapshot-collection mutation in a @Composable body. Given the explicit dedup/scoring rules, a small eval would lock that behavior in.

  5. Intentional but notable drift: evals.json notes compose-agent has no cross-phase / false-lead guidance. Audit-only scope is fine for this PR, but review via compose-agent can still recommend the patterns this release rejects. Worth a follow-up (not blocking 4.3.0).


Nit

  • No docs/release-notes-4.3.0.md while 4.2.0 had dedicated release notes.
  • Positive signal for BoxWithConstraints (parent constraints) sits next to eval 12’s rejection for sibling measured width — already scoped, but one “not for sibling measurement” phrase in the positive bullet would reduce mis-routing.
  • Heuristic scope (“mutation directly in a @Composable body”) will miss mutations via helpers; acceptable tradeoff, worth knowing.

Verdict: Good to merge after tightening CHANGELOG/PR copy; the audit rules themselves look correct and well cross-linked (dedup rules, SSM lambda exemption, eval 12’s BoxWithConstraints guard, diagnostics step 7 scoped to candidate files).

via pr-review-relay · author: claude

…ion eval, 4.3.0 release notes

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ysskXsKpwEbs1n9CX1ScY
@hamen hamen merged commit 3c839fb into main Jul 6, 2026
1 check passed
@hamen hamen deleted the feat/cross-phase-backwrite-false-leads branch July 6, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant