URGENT ADFA-3604: Fix R8 shrink/optimize crashes live on stage - #1609
Conversation
On-device verification of the shrunk release build (requested in review)
surfaced five distinct R8 bugs, each fixed here:
1. R8's optimize pass retargeted a call to CoGo's own isTestMode() to the
unrelated, device-missing dalvik.system.VMRuntime.isTestMode, crashing
every release build on launch. Disabled optimization (-dontoptimize);
shrinking is what actually cuts dex size, optimization was the unsafe
part given the R8/Kotlin 2.3.0 metadata mismatch already on record in
this file (see the existing StopWatch workaround).
2. Protobuf-lite resolves generated message fields by name via reflection;
shrinking removed a "field with no direct bytecode reference"
(SyncMetaModels$SyncMeta.projectModelInfo_), crashing project sync.
3-5. lsp/kotlin's own service registrations (kt-lsp.xml and
AnalysisApiServiceProviders.kt's ::class-literal registrar, which
PicoContainer instantiates reflectively) and Caffeine's runtime cache
implementation selection all hit the same class of bug: a reflective
lookup R8 can't trace from static analysis. Each fix revealed another
instance elsewhere in the same dependency graph, so rather than keep
discovering them one on-device crash at a time, keep the whole
lsp/kotlin runtime dependency graph (org.jetbrains.kotlin, Caffeine,
kotlin-reflect, kotlin-script, coroutines internals, streamex, Trove)
whole.
Verified end to end on a physical device: clean Kotlin analysis session
init, and a live diagnostic ("Expecting member declaration") confirming
the analysis engine correctly parses and resolves code post-shrink.
Net result: release dex settles at ~85 MB (down from the 119.4 MB
baseline), short of the initially-measured 28.8 MB because that number
came from a build that silently corrupted the Kotlin LSP -- this is the
verified-correct number.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
📝 Walkthrough
WalkthroughThe ProGuard configuration disables R8 optimization while keeping shrinking enabled. It preserves Kotlin LSP compiler services, runtime dependencies, and protobuf generated-message fields required for reflection. ChangesR8 and ProGuard preservation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/proguard-rules.pro (1)
45-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce the incident history in this rule comment.
Lines 45-59 describe several crash investigations and runtime details. Keep this comment to the retention reason, the affected dependency graph, and
ADFA-3604. Put the detailed crash history and size trade-off in the ticket or PR description.As per coding guidelines, “Keep documentation, tickets, commit messages, and PR descriptions concise and focused.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/proguard-rules.pro` around lines 45 - 59, Shorten the comment above the broad keep rule to state only the retention reason, that it applies to the full lsp/kotlin runtime dependency graph, and the ADFA-3604 reference. Remove the individual crash investigations, implementation details, and explicit size-tradeoff discussion from this comment.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/proguard-rules.pro`:
- Around line 45-59: Shorten the comment above the broad keep rule to state only
the retention reason, that it applies to the full lsp/kotlin runtime dependency
graph, and the ADFA-3604 reference. Remove the individual crash investigations,
implementation details, and explicit size-tradeoff discussion from this comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 782a2c01-b355-4049-b9e8-78202e6310bd
📒 Files selected for processing (1)
app/proguard-rules.pro
ReviewScope: single file, Findings (all non-blocking; filed as follow-up tickets)
VerdictApprove. Correct, well-scoped, unusually well-documented (each rule ties to a concrete on-device crash trace) fix for a live crash. The trade-offs (dex size, disabled optimization) are honestly disclosed in the PR description and now have tracking tickets. |
* ADFA-5156: Roll back R8 shrinking to unbreak plugins Restores the blanket -dontshrink that ADFA-3604 (#1596) removed on 2026-07-29. This is a temporary rollback to restore plugin functionality; a targeted fix follows. Plugins are loaded parent-first through a stock DexClassLoader (PluginLoader.kt:92-116, parent passed at PluginManager.kt:603), so every kotlin.** class a plugin references resolves from the IDE's dex, not from the ~1058 stdlib classes the plugin bundles. R8 cannot see plugin call sites, so it strips every stdlib member the IDE itself does not call. The net effect is that a plugin can only call the subset of the Kotlin standard library that the IDE also calls; anything else throws NoSuchMethodError at runtime. Sketch to UI fails on every image load with "No static method maxOrNull([F)Ljava/lang/Float; in class ArraysKt". -dontobfuscate and -dontoptimize were already set, so restoring -dontshrink reduces R8 to a pass-through and returns the release build to the configuration shipped before ADFA-3604. isMinifyEnabled and isShrinkResources are deliberately left alone, keeping resource shrinking and the build wiring unchanged. Verified by dex-scanning both APKs (baseline pulled from a release install on Samsung RFCT704HEAL): kotlin/kotlinx method declarations 30,669 -> 45,371 ArraysKt/CollectionsKt/MapsKt/ FilesKt/SequencesKt facades absent -> present 10 sketch-to-ui stdlib call sites all stripped -> all present CompletableJob$DefaultImpls.plus stripped -> present Sketch to UI now loads an image and completes detection on-device with no NoSuchMethodError in logcat. APK size: 659,307,160 -> 706,829,817 bytes (+47.5 MB, +7.2%). Note: R8 was buying less than #1596 advertised. That PR measured dex at 28.8 MB / 24,853 classes, but the shipped APK is 85 MB / 78,757 classes -- the URGENT follow-ups (#1609, #1610) added -dontoptimize plus a set of keep rules that clawed most of it back. * ADFA-5156: Add R8 plugin-impact analysis tooling The ADFA-5156 failure mode is invisible at build time -- assemblePlugin is green, the manifest is fine, the .cgp is correct, and only on-device execution of a specific code path reveals that R8 stripped a stdlib member the plugin needs. These scripts make it measurable from build artifacts instead. scripts/r8-plugin-impact/ README.md what the bug is, how to run, how to read output, the known false positives, and the ADFA-5156 baseline numbers to measure future builds against dex-dump.sh extract + dexdump an APK or .cgp analyze-plugin-impact.py simulate parent-first resolution of every kotlin.*/kotlinx.* call site in each plugin's own code against two host dexes and diff the verdicts Three subcommands: impact (the before/after table), explain-method (trace one resolution chain, showing where it leaves the APK), explain-absent (inspect fall-throughs for the split-brain shape that caused the original bug). Documents three traps that produce wrong conclusions if analysis is done ad hoc, all of which bit during this investigation: - Methods inherited from the Android boot classpath read as missing, because java.util.* is not in the APK. Eight such false positives are enumerated. - Kotlin multifile facades (ArraysKt, StringsKt) declare nothing themselves; they extend a part class whose underscore count varies (StringsKt__StringsKt vs ArraysKt___ArraysKt). Checking a facade directly always fails. - D8 build-time synthetics ($$ExternalSyntheticBackport0 and friends) never exist in the host and always show as absent. Stdlib-only Python, no third-party dependencies. Run with uv run --no-project. * ADFA-5156: Apply spotless formatting to plugin-impact scripts * ADFA-5156: Point plugin-impact baseline at the deployed plugin set The first baseline measured a local folder of .cgp files that turned out to be a pre-rename snapshot -- 5 stale filenames and 3 plugins missing. Re-runs against the artifact from the last update-libs.yml deploy (26 plugins, 4,261 call sites) and documents how to obtain that artifact, so the next person does not measure the wrong set. Conclusion is unchanged: zero regressions, 67 real failures to 0.
🚨 Urgent: fixes a release-build crash currently live on
stagePR #1596 (ADFA-3604) merged to
stageon 2026-07-29 before on-device verification. On-device testing (done afterward) found that the merged version crashes every release build immediately on launch:R8's optimize pass silently retargets a call to Code On the Go's own
isTestMode()utility to the unrelated, device-missingdalvik.system.VMRuntime.isTestMode(), because it fails to parse Kotlin 2.3.0 metadata correctly (see the pre-existingStopWatchworkaround comment inapp/proguard-rules.profor the same root cause hit before). Anyone building a release APK from currentstagewill hit this.What this PR fixes
On-device testing surfaced seven distinct R8 shrink/optimize bugs. All are fixed here:
isTestMode()toVMRuntime.isTestMode(). Fixed with-dontoptimize; shrinking (dead-code removal) is what actually cuts dex size, optimization was the unsafe part given the R8/Kotlin version mismatch.SyncMetaModels$SyncMeta.projectModelInfo_, crashing project sync.lsp/kotlinregisters services via::classliterals (AnalysisApiServiceProviders.kt) that PicoContainer instantiates reflectively; R8 stripped "unused" no-arg constructors one at a time as each was discovered (ClassNotFoundException, thenPicoInitializationException). Fixed by keeping the wholelsp.kotlin.compiler.servicespackage.NullPointerExceptiondeep in IntelliJ'sJavaCoreApplicationEnvironmentbootstrap — verified absent on an unshrunk debug build, confirming it's shrink-caused, not environmental.gson.fromJson(..., X::class.java)reflection have no traceablenewcall site, so R8 strips their constructor and Gson's runtime then reports them as abstract (Failed to load template archive ... Abstract classes can't be instantiated!onTemplatesIndex). Found and fixed for everygson.fromJsoncall site in the repo, including two more instances with no prior keep rule at all (OpenedFilesCache/OpenedFile, breakpoint persistence models).com.sun.tools.jdi'sSocketAttachingConnector/SocketListeningConnectorare loaded viaServiceLoader, which R8 can't trace; stripping their constructors broke the debugger withError: no Connectors loaded, which the app then surfaced to the user as a misleading "Network access error" (the debug-connect failure handler always appends a network-access suggestion regardless of actual cause). This exact fix was already anticipated and left commented out in this file since before shrinking was ever genuinely enabled — just needed uncommenting.Each fix revealed another gap elsewhere in the same dependency graph, so rather than keep discovering them one on-device crash at a time, this keeps the whole
lsp/kotlinruntime dependency graph whole (org.jetbrains.kotlin, Caffeine, kotlin-reflect, kotlin-script, coroutines-internal, streamex, Trove).Verification
Built and installed on a physical ARM device (Samsung Galaxy Note20 Ultra):
KotlinLanguageServer: Kotlin project initializedwith no errors (previously:NullPointerException/ClassNotFoundException/PicoInitializationException/IllegalStateException, one per fix)Starting JDWP listener,startListening), no dialogFATAL EXCEPTIONin logcat across the whole sessionSize impact
Release DEX settles at ~85 MB (down from the 119.4 MB pre-ADFA-3604 baseline). This is short of the 28.8 MB originally reported on #1596 — that number came from a build that silently corrupted the Kotlin LSP; this is the verified-correct number.
Test plan
🤖 Generated with Claude Code