ADFA-3604: Fix 2 more R8 shrink bugs (Gson templates, JDI debugger) - #1610
Merged
Conversation
Found via a "Failed to load template archive ... Abstract classes can't be instantiated!" error on-device. Same root cause as the other shrink bugs fixed in this branch: Gson deserializes these classes only via reflection (gson.fromJson(..., X::class.java)), never a direct `new` R8 can trace, so it strips the constructor and Gson's runtime then reports the class as abstract. Fixes templates-impl's TemplatesIndex/TemplateJson/etc., and two more instances found by auditing every gson.fromJson call site in the repo: OpenedFilesCache/OpenedFile (no prior rule at all) and the breakpoint persistence models. Verified on-device: templates load cleanly, zero FATAL EXCEPTION in logcat, Kotlin project init still succeeds. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Found via a "Network access error" dialog on-device -- misleading, since
the app's debug-connect failure handler always appends a network-access
suggestion regardless of actual cause. The real error was buried in
logcat: "java.lang.Error: no Connectors loaded" from
com.sun.tools.jdi.VirtualMachineManagerImpl, caused by a
ServiceConfigurationError failing to instantiate
SocketAttachingConnector/SocketListeningConnector. Same root cause as
the other shrink bugs: JDI loads these via ServiceLoader, which R8
can't trace, so it stripped their no-arg constructors.
This exact fix was already anticipated and left commented out in this
file ("Initial rules to enable when R8 is shrinking to address
exceptions") from before shrinking was ever genuinely enabled -- just
needed uncommenting now that it is.
Verified on-device: JDWP listener starts successfully, no dialog, zero
FATAL EXCEPTION in logcat.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 Walkthrough
WalkthroughThe ProGuard configuration now keeps Gson-deserialized model packages and template ZIP models. It also enables retention for JDI connector classes and constructors loaded through ServiceLoader. ChangesShrinker retention rules
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@app/proguard-rules.pro`:
- Around line 117-118: Correct the positional reference in the comment
describing templates.impl.zip near the OpenedFilesCache/OpenedFile rules: change
“above” to “below” to reflect the template rule’s location, or remove the
positional wording entirely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d45ea7d7-01a5-4824-b9f3-3f25873e5b37
📒 Files selected for processing (1)
app/proguard-rules.pro
The Gson keep-rule comment said the templates.impl.zip rule was "above" when it's actually below in the file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
hal-eisen-adfa
added a commit
that referenced
this pull request
Aug 15, 2026
* 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1609 — two more R8 shrink bugs found after that PR merged
#1609 (fixes 1–5 of the R8 shrink/optimize bugs found while verifying ADFA-3604 on-device) was merged to
stagewhile this session was still actively testing. This PR carries the two additional fixes found afterward, cherry-picked cleanly onto currentstage.What this PR fixes
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, surfaced as a user-facing error dialog). 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.Verification
Built and installed on a physical ARM device (Samsung Galaxy Note20 Ultra):
Starting JDWP listener,startListening), no dialogFATAL EXCEPTIONin logcatBroader smoke test — done
The smoke test recommended below (build/run a project, Java LSP, XML LSP) is now complete, on a release build with all 7 fixes applied:
Toast.completion returned correct members (Callback,LENGTH_LONG,LENGTH_SHORT) with correct types/values.android:__removed0/1/3/4placeholder entries — but this traces to genuine AOSP placeholder attrs baked into the bundled platform'sattrs.xml(Google's own reserved slots for deprecated attribute IDs), not to R8/shrinking. R8 never touches bundled asset files, only app bytecode, so this predates ADFA-3604 and reproduces on any build. Filed separately as ADFA-4980 rather than folding it in here.No further R8 shrink/optimize bugs found. With this PR merged, ADFA-3604's fix set is fully verified on-device across build, Java LSP, Kotlin LSP, XML LSP, templates, and the debugger.
🤖 Generated with Claude Code