ADFA-4128 (11/11): app + bench — wiring Quick Build into the IDE and the benchmark harness

[fryanpan]

Part 11/11 of the stacked split of #1669 (requested by Akash). Base: feature/ADFA-4128-qb-10-gradle-plugin. Stack overview + review mechanics: PR 1 (#1713). Terms are defined in quickbuild/README.md (lands in PR 1).

Puts Quick Build in front of the user: a button next to Run, and enough narration to tell what it is doing and when it has finished. It also adds a harness to make it easier to run standard Gradle build and Quick Build benchmarks, and to gather key metrics about the stages of the build process.

flowchart TB
    subgraph appc["<b>This PR: inside app/ — wiring and bench</b>"]
        act["QuickBuildAction<br/>registered only when<br/>FeatureFlags.isExperimentsEnabled<br/><i>QuickBuildAction.kt</i>"] --> mgr["QuickBuildManager<br/>session lifecycle, provisioning,<br/>stop-tap cancellation"]
        mgr --> narr["QuickBuildOutputNarrator<br/>attached to the session manager;<br/>queues while no pane is bound<br/><i>QuickBuildOutputNarrator.kt</i>"]
        mgr --> sb["status bar collector<br/>lifecycle-scoped: state, not history<br/><i>QuickBuildStatusBar.kt</i>"]
        koin["QuickBuildModule (Koin)<br/>binds every core port;<br/>assetsLiveReloadable read once<br/>at the Android edge<br/><i>QuickBuildModule.kt</i>"]
        tr["bench trampoline activity<br/>debug-source-set manifest only<br/><i>QuickBuildBenchActivity.kt</i>"] --> mgr
        mgr --> hooks["QuickBuildBenchHooks<br/>inert release twin<br/><i>debug/QuickBuildBenchHooks.kt</i>"]
        hooks --> rec["event + metrics recorders"]
        rec --> log["bench-events.jsonl<br/><i>BenchEventsFile.kt</i>"]
        hooks --> e2e["MODE_STANDARD<br/>autostarts the standard Run and<br/>stops at the build result<br/><i>QuickBuildBenchAutostart.kt</i>"]
    end
    adb["adb shell am start<br/>gated on android.permission.DUMP"] --> tr
    mgr --> core[":quickbuild:core session manager (PRs 5-8)"]
    narr --> pane["Build Output pane (existing)"]
    sb --> bar["bottom status bar (existing)"]
    mgr -- "provisioning + rebuild builds" --> gbs["GradleBuildService (existing)"]
    classDef thisPrBox fill:#dbeafe,stroke:#93c5fd,color:#1e3a5f
    classDef inPr fill:#ffffff,stroke:#64748b,color:#000
    class appc thisPrBox
    class act,mgr,narr,sb,koin,tr,hooks,rec,log,e2e inPr

Loading

What to review

• QuickBuildAction.kt — owns the session tap: start, stop-tap cancel, grey-out. Line-by-line.

• Gradle tuning — Metaspace 192→384 MB + daemon idle timeouts (30 min balanced / 2 h high-perf); the only changes to non-QB behavior

• QuickBuildOutputNarrator.kt, QuickBuildStatusBar.kt — queued narration; lifecycle-scoped status showing state, not history.

• QuickBuildModule.kt — binds every core port; reads assetsLiveReloadable at the Android edge.

• GenerateSourcesDeferral.kt — defers resource-XML generateSources until Quick Build goes idle.

• John's items C15, C16, C17, C22, C23 folded in as fixes.

• Rollback: without the flag there is no UI entry point.

• Followup, not fixed: R8 emits kotlin.Metadata warning noise.

• QuickBuildBenchAutostart.kt — MODE_STANDARD autostarts the standard Run and stops at the build result, so it measures the build only. Line-by-line.

• The standard arm's install dialog is suppressed, because an unattended run cannot answer it. Quick Build's arm measures build, deploy and reload, so the two are not like for like from in-app numbers alone.

• QuickBuildBenchActivity.kt, QuickBuildBenchHooks.kt — DUMP-gated trampoline; inert release twin.

• BenchQuickBuildMetricsSink.kt — relaunchOk is always recorded; toRunningMillis rides only on a relaunch that reconnected, so it is absent rather than a measured zero.

How this PR Was Tested

• Automated tests (see coverage details below)

• Manual QA — walked the manual-qa.md test plan on the A56 [measured on a56]

• Benchmark — measured on real devices, both arms: a warm code edit reaches the running app with about a 5x median speedup over a standard build + deploy (A56 4.35x, A06 6.53x, 5.12x combined, from the pass accepted on 2026-08-11). The weaker the phone, the bigger the win.

  How the two arms were made comparable, since they are not like for like from in-app numbers: the in-app standard arm (MODE_STANDARD) suppresses the install dialog and stops at the build result, so it measures the build only. The benchmark harness adds install and launch from outside the process — it drives the install dialog and takes the span from the save to the app's Displayed on the device clock — and the published basis subtracts the harness's own time from that span. Quick Build's arm measures build, deploy and reload in-app. There is no MODE_STANDARD_E2E; earlier drafts of this description named one that was never in the tree.

• Still open — the rebaseline relaunch path is not yet device-verified, and neither is the API 28/29 resource-swap success path.

Coverage (JaCoCo at the stack tip, single run):

A lot of this was UI code and wasn't covered very well by automated tests.

Package	Line	Branch	Note
actions/build	0.5%	0.0%	UI — device-tested
actions/file	0.0%	0.0%	UI — device-tested
activities/editor	0.9%	1.8%	UI — device-tested
analytics/quickbuild	49.6%	30.4%
app	0.7%	0.5%	Application classes, Android-bound
di	0.0%	0.0%	Koin wiring
fragments/sidebar	0.0%	0.0%	UI — device-tested
handlers	7.6%	0.0%	Android-bound
quickbuild	27.3%	24.3%	mixed UI/logic; see caveat
services/builder	6.8%	0.0%	bound Service, Android-bound
utils	0.0%	0.0%	mixed UI/logic
viewmodel	0.9%	0.0%	UI — device-tested
TOTAL (all app files in this PR)	16.9%	15.3%	2,989 lines, 1,582 branches

Review fixes (2026-08-22)

A review-fixes commit addresses the code-review findings. Two changes here deliberately ship to all users, with the Experiments flag off (approved):

• GradleBuildTuner. Benefit: builds stop dying with out-of-memory errors on ordinary phones, and a rebuild shortly after a build is much faster. Two settings on every Gradle build: (a) the Metaspace cap (JVM memory for loaded class definitions) goes 192 MB -> 384 MB -- real Android-plugin builds exceed 192 and were being killed mid-build; it's a cap, not a reservation, so no extra memory is used unless the build needs it. (b) The Gradle daemon (the background process that keeps builds warm) stays alive for a time matched to the phone's tier -- 15 min low-memory, 30 min balanced, 2 h high-performance -- so a quick rebuild skips the cold start while weak phones don't host a resident daemon for hours.
• generateSources narrowing. Benefit: fewer surprise build stalls while editing, less battery and CPU burned. The IDE used to launch a Gradle generate-sources step after every save-all and any XML save, even when nothing that step produces could have changed. Now it runs only when the saved file can actually affect generated sources. Same results, far fewer builds.

One candidate followup from review (orchestrator forcing a full-changed compile after a failed dex/deploy) was re-checked and refuted at this tip: the forced flag re-arms and a forced no-op already performs the full rebuild. The daemon-side recovery lever stays in as defense in depth.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

[claude]

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.}

[fryanpan]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review skipped: 106 files exceed the limit of 100.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[jatezzz]

@fryanpan — review of the Quick Build app wiring. Seven findings; two are worth fixing before merge and are left as inline comments:

• ProjectHandlerActivity.kt:549 — the reArmInstall safety net does not cover the clobber dialog, so a rotation while it is up loses the install silently.
• QuickBuildStatusBar.kt:151 — a landed build is re-announced on every re-subscribe and, with onlyIfOwned = false, stomps project-init / plugin-install status.

The remaining five are low: the session teardown on a transient APK parse failure, the null-activity path that builds stale content, the ellipsized actionable status copy, and three unused imports that should fail spotlessCheck.

---

One more (low), which could not be left inline because the file is not in this diff:

app/src/main/java/com/itsaky/androidide/actions/BaseBuildAction.kt:43 — a missed sibling of this PR's raw-vs-user-visible split.

This PR moved every UI decider over to isUserVisibleBuildInProgress — AbstractCancellableRunAction, ProjectHandlerActivity.onResume, the progress bar, BuildVariantsFragment — but BaseBuildAction.prepare still reads the raw flag:

enabled = buildService?.let { !it.isBuildInProgress } == true

With the experiments flag on, Quick Build's eager prebuild now runs on every project open, so RunTasksAction (and any other direct BaseBuildAction) is silently greyed out for its whole duration with no explanation — unlike QuickBuildAction, which relabels, or QuickRunAction, which flashes msg_build_slot_busy. Note that AbstractCancellableRunAction.prepare unconditionally re-sets enabled = true, which is why its new slot-busy flash is reachable and these are not.

---

Checked and cleared: all new string/drawable/menu resources resolve on the head branch; ProjectManagerImpl.generateSources() returns Boolean, so GenerateSourcesDeferral's refusal-retry contract holds and a throw is treated as a refusal rather than cancelling the scope; InternalBuildBracket releases strictly after isBuildInProgress clears, so there is no window where isUserVisibleBuildInProgress reads true for an internal build; QuickBuildOutputNarrator confines its mutable state to one Dispatchers.Main.immediate scope and compares sink identity correctly; QuickBuildReloadTimingMetric.asBundle() is 25 params worst case, within the Firebase cap; ThermalSafeStrategy copies GradleDaemonConfig, so the new non-defaulted daemonIdleTimeoutMs is safe and 2h fits in Int; and InstallationResultHandler.onResult returning null is handled as "do nothing" by its only caller.

[jatezzz]

@fryanpan low — unused import.

Position appears only on this import line; the identifier is never used in the file. Same standard:no-unused-imports / Spotless-ratchet issue as the two EventBus imports added to ProjectHandlerActivity.kt.

[itsaky-adfa]

NITPICK: still open at head - Position occurs exactly once in the file, on this import line.

Worth knowing why the tooling misses it: the file contains 47 occurrences of the substring Position (tabPosition, fromPosition, ensurePositionVisible, ...), and ktlint's unused-import detection is substring-based, so the identifier reads as used. spotlessApply will not remove this one for you.

[fryanpan]

Not deleted, and the reason is worth knowing. It was unused at the head you reviewed. The stack has since been rebased onto stage, and stage's deep-link work (#1651) added a Position(line, column) call in this same file — so the import is live again and deleting it breaks the build. Leaving it.

[fryanpan]

All round-3 comments addressed; ready for another look.

[itsaky-adfa]

Second full pass at 2748c9ae, effort high. Static only: no build, no Gradle, no device, no emulator was run for this review (five agents share one read-only worktree). Everything below was verified by reading the tree at the head SHA.

Governing document. REVIEW.md states in its first line that it is "a coaching doc, not a gate" and sets no approve/request-changes threshold, so the operative rule is CLAUDE.md's status progression: a review with no outstanding critical, high or medium findings moves the ticket to QA. Three IMPORTANT findings are outstanding, all on the flag-off path, so this stays in code review.

Re-check of the 17 prior threads. Twelve are genuinely fixed at head; four are still live; one is fixed only in part.

Fixed, verified by reading the code at head - not by the reply:

• ProjectHandlerActivity.kt (orig 549) dispatched = true before the clobber dialog - fixed. The dialog is now awaited (awaitBuildTypeSwitchConfirmation, suspendCancellableCoroutine) and dispatched = true is set only after a decision is reached, so a rotation under the dialog falls through to finally { reArmInstall(state) }.
• QuickBuildStatusBar.kt (orig 151) landed build re-announced on re-subscribe - fixed. upToDateUpdate's previous == null -> null branch is now first, ahead of the buildDurationMillis != null branch (QuickBuildStatusBar.kt:177).
• ProjectHandlerActivity.kt (orig 560) transient APK parse failure tearing down a healthy session - fixed. The restart is now gated on now is QuickBuildClobberConfirmation.Needed, so NeededForUnknownAppId no longer restarts.
• QuickBuildAction.kt (orig 69) null-activity path skipping the save-all flush - fixed. val activity = data.getActivity() ?: return false now precedes everything (QuickBuildAction.kt:71).
• layout_editor_build_status.xml (orig 20) actionable copy truncated at 2x - fixed. maxLines is 3, the container is wrap_content with editor_sheet_collapsed_height as a minHeight floor, EditorBottomSheet.refreshCollapsedHeight() grows the header to the measured text, and the actionable strings were shortened (quick_build_status_app_not_running is now 47 chars). See the residual note below.
• ProjectHandlerActivity.kt (orig 140) unused Subscribe/ThreadMode imports - fixed; neither is present at head.
• EditorHandlerActivity.kt:93 unused Position import - fixed; Position is used at EditorHandlerActivity.kt:3176.
• EditorHandlerActivity.kt (orig 625) ungated long-press dropdown - fixed. showQuickBuildDropdownMenu now reads title = quickBuild.label and isEnabled = quickBuild.enabled onto the item and the click handler gates on quickBuild.enabled (EditorHandlerActivity.kt:831-843).
• EditorHandlerActivity.kt:1325 the third generateSources call site - fixed. The plugin save path at EditorHandlerActivity.kt:1431 now reads if (result.resourceXmlSaved) { GenerateSourcesDeferral.notifyResourceSaved() }.
• QuickBuildBenchAutostart.kt:30 MODE_STANDARD_E2E in the description - fixed. git grep STANDARD_E2E at head returns nothing, and the body now says so explicitly and re-labels the mermaid node MODE_STANDARD.
• ApkInstaller.kt (orig 60) dead requestDowngrade - fixed. grep -rn requestDowngrade app/src at head returns nothing; the parameter and its plumbing were deleted (commit 3dfbaf16a).
• BenchQuickBuildMetricsSink.kt:116 wrong field named in the description - fixed; the body now reads "relaunchOk is always recorded; toRunningMillis rides only on a relaunch that reconnected".
• QuickBuildStatusBar.kt:88 StartFailed re-writing over another writer - fixed, though not by the suggested onlyIfOwned = true. QuickBuildStatusTracker now lives on the activity rather than inside the repeatOnLifecycle block, so on a re-subscribe previous == current and quickBuildTransition returns None instead of StartFailed.

Still live - each gets a reply in its own thread rather than a new one: the bench cross-project autostart (QuickBuildBenchActivity.kt:105), the bench onCreate main-thread runBlocking (QuickBuildBenchActivity.kt:52), the reflowed GPL header (layout_editor_build_status.xml:1), and - partly fixed - the main-thread resolve of the session manager (ProjectHandlerActivity.kt, orig 662).

Five threads are currently unresolved but fixed and can be closed: EditorHandlerActivity.kt:93, QuickBuildBenchAutostart.kt:30, ApkInstaller.kt (orig 60), BenchQuickBuildMetricsSink.kt:116, QuickBuildStatusBar.kt:88. Nothing needs unresolving - every thread already marked resolved was verified fixed.

Architecture pass (architecture-review skill, section 10; ARCHITECTURE.md + ADRs 0001/0003/0005/0006/0007/0009 read at head):

Verdict	Rule	Source	Evidence
pass	Koin DI, constructor injection, registered in a module	ADR 0006	quickBuildModule registered in IDEApplication.kt:255; every port constructor-injected; no hand-rolled singleton added
pass	Room vs raw SQLite	ADR 0001	no persistence added. The only new store is PreferencesQuickBuildHistoryStore (one boolean per project in SharedPreferences) - non-relational settings, which the policy assigns to preferences
pass	Strings in :resources	REVIEW.md section 7	every new string lands in resources/src/main/res/values/strings.xml; a sweep of added lines for flashError/flashInfo/setStatus/setTitle/setMessage/contentDescription with a literal argument found zero hardcoded user-facing text
pass	ABI flavors, no flavorless task	ADR 0005	androidComponents.onVariants derives per-variant task names from variant.flavorName; no per-module flavor block added
pass	No new dependency	ADR 0003 / ARCHITECTURE.md	only implementation(projects.quickbuild.core) - an in-tree project, no Maven coordinate
pass	@Parcelize	ARCHITECTURE.md, Parceling	no Parcelable added
pass	System bars not drawn over or intercepted	CLAUDE.md	no WindowInsets/systemBars/fitsSystemWindows change anywhere in the diff; refreshCollapsedHeight keeps the existing collapsedHeight + insetBottom header contract
pass	a11y and long-press help on new interactive elements	REVIEW.md sections 8-9	cd_quick_build, cd_quick_build_error, cd_quick_build_slow, cd_quick_build_reconnecting wired per tone in EditorHandlerActivity.getToolbarContentDescription; QuickBuildAction.retrieveTooltipTag returns EDITOR_TOOLBAR_QUICK_BUILD and the dropdown carries its own Help item
pass	Benchmark harness out of the release build	REVIEW.md section 14	QuickBuildBenchActivity/BenchStateRecorder/BenchEventsFile/BenchQuickBuildMetricsSink are all in app/src/debug/, declared only in app/src/debug/AndroidManifest.xml (exported but android:permission="android.permission.DUMP"), and app/src/release/.../QuickBuildBenchHooks.kt is an inert twin with isEnabled = false and standardBuildEnded(...) = false. The assets are a separate matter - see the inline finding on app/build.gradle.kts
warning	New dialogs and the split-button dropdown are XML/View, not Compose	ADR 0009	posted inline as a NITPICK
warning	Mutually-exclusive states as independent booleans	ARCHITECTURE.md, State Management	EditorViewModel gains _isInternalBuildInProgress alongside _isBuildInProgress; the two are mutually exclusive by construction (isUserVisibleBuildInProgress = isBuildInProgress && !internalBuild.isHeld), so a sealed build-state would model them without a contradictory pair. Not posted separately - it is one more LiveData flag on a legacy ViewModel the doc already exempts from migration, and the sealed BuildState next door is the eventual home
warning	Cross-project Gradle configuration reach-through	ARCHITECTURE.md, "All module config flows through composite-builds/build-logic"	app/build.gradle.kts uses evaluationDependsOn(":quickbuild:runtime")/(":quickbuild:daemon") and then reads daemonProject.tasks.named(...) and daemonProject.configurations.named("runtimeClasspath") directly. Harmless today (this repo runs without the configuration cache and without isolated projects) but it is the pattern both features forbid, and the staging logic would sit more naturally in build-logic. Recorded here rather than inline - it is a note about where the code lives, not a defect
pass	Module boundary direction	ARCHITECTURE.md	app -> quickbuild:core only; nothing in quickbuild/* gained a dependency on app

Font-scale review is static and therefore incomplete. No device or emulator was used, so nothing here was verified at font scale 1.0 or 2.0 - the PR's own claim to have walked manual-qa.md on an A56 is not something this review can confirm or contradict. Statically the changed surface reads correctly: text is sp, spacing is dp, no sp dimen is used as a margin or padding, the previously fixed 100dp box is now a minHeight floor plus a measured grow, and maxLines went 1 -> 3 on the one clamped TextView. One residual, PLAUSIBLE: quick_build_status_deploy_failed ("Quick Build: built, but could not be delivered - see Build Output", 65 characters) against maxLines="3" at 2x on a 360dp-wide phone is roughly at the three-line boundary by arithmetic (about 23 characters per line at BodyMedium doubled), and any longer translation of it goes over. Worth one screenshot at 2.0 with that specific string on the bar during QA; not posted as a finding because I could not measure it.

Coverage. Reported at 16.9% line / 15.3% branch overall. Most of that is legitimately exempt UI, and the PR says so per package. The exception is posted inline.

Size. 120 files and +13582 lines is far past the ~500 LOC / ~10 file signal in CLAUDE.md and REVIEW.md section 14, but it is not recorded as a finding: this is deliberately the tip of an 11-part stack, the 26 commits are genuinely reviewable individually, and the two whole-file Spotless reformats were correctly split into standalone style: commits (97a3cb5af, f8eb25706). The structure the rule asks for is present. One file slipped that discipline - posted inline as a NITPICK.

No finding was dropped for want of an anchor, and no nitpick was shed to the volume cap (8 inline + 4 replies, against a cap of 15 with at most 5 NITPICK).

[itsaky-adfa]

IMPORTANT: every Gradle progress event now forces a layout pass on the bottom sheet header, for all users with the Experiments flag off.

setStatus ends in post { refreshCollapsedHeight() } (line 679), and refreshCollapsedHeight calls header.requestLayout() here - before the measured == measuredStatusHeight early-out - so the request fires whether or not the height changed. Its hottest caller is EditorBuildEventListener.onProgressEvent, which calls setStatus(event.descriptor.displayName) on every ProjectConfigurationStartEvent and TaskStartEvent: hundreds of off-pass measure() plus requestLayout() round-trips per build, on the main thread, during the app's heaviest moment. None of this is behind the Experiments flag - it is the standard build's own task-line narration.

Move header.requestLayout() below the early-out so it runs only when the measured height actually changed.

[itsaky-adfa]

IMPORTANT: a manifest-only save no longer refreshes the generated sources, for every user, with the Experiments flag off - and the PR body says the opposite.

resourceXmlSaved is modified && isXml && isAndroidResource(), and ProjectManagerImpl.isAndroidResource matches only files under module.getResourceDirectories(). AndroidManifest.xml sits at src/main/AndroidManifest.xml, outside all of them, so it can never set the flag - where xmlSaved did. Edit the manifest to add a custom <permission>, save, and Manifest.permission.MY_PERM stays unresolved in the editor until the next resource save or a full build.

The comment above owns the trade ("a manifest-only edit no longer refreshes the generated Manifest/R intermediates"), but the PR body's review-fixes section reads "Same results, far fewer builds" - so QA has no reason to test a manifest edit. Either restore the manifest case (fileName == "AndroidManifest.xml" alongside the resource check) or correct the body to state the regression, and add it to Steps to QA.

[itsaky-adfa]

IMPORTANT: this stages the ~62 MB Quick Build daemon payload into every variant's assets, release included, with no mention of it anywhere in the PR.

androidComponents.onVariants { variant -> ... } has no selector(), so copy<Variant>QuickBuildDaemonZip and copy<Variant>QuickBuildRuntimeAar run for release as well as debug. The zip is the daemon jar plus its entire runtimeClasspath (kotlin-build-tools-impl, the embeddable Kotlin compiler) plus the Compose compiler plugin - quickbuild/daemon/build.gradle.kts puts it at "~62 MB" in its own size audit - and the feature consuming it is opt-in behind FeatureFlags.isExperimentsEnabled. Every user pays the download and the on-device storage; ADR 0005 exists because this project treats APK size as a first-order constraint.

Either gate the two addGeneratedSourceDirectory calls the way line 714 already gates a release-only block, or state the measured release-APK delta in the PR body so the release owner can make the call deliberately. I did not build an APK, so the exact delta is unverified.

[itsaky-adfa]

MINOR: analytics/quickbuild is fully off-device code and lands under REVIEW.md's coverage bar on both axes.

REVIEW.md section 5 asks new non-UI code for >= 50% line and branch, exempting only Composables/Activities/Fragments. The PR's own table puts this package at 49.6% line / 30.4% branch. Nothing here is Android-bound: the clock, the project path and the module count are all injected, and every callback is a pure map from a domain type to a metric. onInvalidation, onProxyAppRebuild and the elapsedMs fallback in onBuildFinished (the started == null branch, which decides whether a duration is real or -1) are the branches worth pinning.

No runtime defect - it is the change's safety net rather than the change. A few cases over onBuildFinished/onReloadTimeline with a fake IAnalyticsManager would clear the bar.

[itsaky-adfa]

MINOR: this activity is now 2003 lines and owns seven separate Quick Build concerns.

+778 lines here add: session-manager and narrator resolution plus their flag gating, five collectors, the Build Output bind/unbind, status-bar ownership, the install-time clobber re-check with three dialog builders and a suspending dialog adapter, the benchmark autostart claim and fire, the prebuild stagger, and the external-build hand-back. ARCHITECTURE.md asks for business logic out of Activities and REVIEW.md section 7 for one owner per concern.

Credit where it is due - every decidable piece is already extracted and tested (QuickBuildClobberConfirmation.kt, SaveResultFlags.kt, QuickBuildStatusBar.kt, QuickBuildTransitions.kt, QuickBuildFlashes.kt), so nothing computes a wrong answer today. The cost is on the next change: the install/clobber block (roughly lines 545-700) is a coherent unit that would move behind a collaborator holding the ViewModel and the dialog host, leaving the activity to forward lifecycle.

[itsaky-adfa]

NITPICK: the Spotless whole-file rewrite of this file is inside the behavioural commit, not a standalone style: one.

+127/-108 on a 108-line file is a line-ending and indentation rewrite under the ratchet; the real change is three lines (xmlSaved -> resourceXmlSaved, routed through GenerateSourcesDeferral). git log -- app/.../SaveFileAction.kt shows only dc4cc3f186 and ceb57092bb, both behavioural, so a reviewer reading by commit cannot see the three lines that matter.

CLAUDE.md asks for the reformat as its own style: spotless reformat, no functional change commit - which this PR already does correctly for 97a3cb5af7 and f8eb25706. Splitting it the same way would make the real diff visible; not worth a rebase on its own if nothing else needs one.

[itsaky-adfa]

NITPICK: this string calls the product "Code on the Go" while quick_build_switch_unknown_app_message four lines below calls it "Code On The Go".

The second is the form CLAUDE.md and ARCHITECTURE.md use. Both strings feed Crowdin, so the inconsistency ships into every locale and translators have no way to know which is intended.

[itsaky-adfa]

NITPICK: three new AlertDialogs and a new XML-menu PopupMenu, where ADR 0009 asks for Compose.

ADR 0009 names dialogs and reusable components explicitly: new ones are composables. This adds showProxyAppWontStayUpDialog, showBuildTypeSwitchDialog/awaitBuildTypeSwitchConfirmation, and menu_quick_build.xml behind a PopupMenu.

Flagging for the record rather than asking for a change: the host is the existing View-based editor toolbar, ADR 0009 keeps existing View surfaces as-is, and the long-press 3-tier help the dropdown's Help item routes into has no Compose entry point until ADFA-4381 lands. Worth a line in the PR body saying that is the reason, so the next reader does not have to re-derive it.

[itsaky-adfa]

All three IMPORTANT findings ship to every user with the Experiments flag off, so none of them is gated by the opt-in:

1. EditorBottomSheet.kt:483 - refreshCollapsedHeight() calls header.requestLayout() before its own measured == measuredStatusHeight early-out, and setStatus() posts it. Its hottest caller is EditorBuildEventListener.onProgressEvent, which fires on every ProjectConfigurationStartEvent and TaskStartEvent: hundreds of forced main-thread layout passes per Gradle build. Move the requestLayout() below the early-out.
2. EditorHandlerActivity.kt:1324 - the generateSources narrowing drops AndroidManifest.xml saves, because isAndroidResource matches only files under module.getResourceDirectories(). The code comment owns the trade, but the PR body's review-fixes section says "Same results, far fewer builds" - which is why QA has no reason to test a manifest edit. Either restore the manifest case or correct the body and add it to Steps to QA.
3. app/build.gradle.kts:469 - androidComponents.onVariants carries no selector(), so the ~62 MB quickbuild-daemon.zip and the runtime AAR are staged into release assets too, unmentioned anywhere in the PR. Line 714 of the same file already shows the withBuildType("release") idiom. Either narrow it or state the measured release-APK delta so the release owner can decide deliberately.

Also outstanding, and worth a look while you are in here: four prior threads are still live (the unsequenced main-thread resolve, the bench cross-project autostart, the bench onCreate main-thread work, and the reflowed GPL header), and analytics/quickbuild sits under REVIEW.md section 5's coverage bar on both axes.

Under CLAUDE.md's status progression an outstanding medium finding keeps this out of QA. REVIEW.md is explicitly a coaching doc rather than a gate, so that progression rule is what this verdict rests on.

Scope of the review, so nothing here is taken for more than it is: this pass was static only - no build, no Gradle, no device, no emulator. Every font-scale conclusion is therefore unverified by measurement, including the one residual risk called out in the review body (quick_build_status_deploy_failed, 65 characters, against maxLines="3" at 2x on a narrow phone). The PR's own claim of a manual-QA walk on an A56 is neither confirmed nor contradicted by this review. The release-APK delta in finding 3 is likewise unmeasured; the "~62 MB" is quoted from quickbuild/daemon/build.gradle.kts's own size audit.

[jatezzz]

@fryanpan medium — the debugger's status line bypasses the ownership flag, and Quick Build then overwrites or blanks it.

The KDoc right above says "Whoever writes the bar owns it", but this is the only site that clears ownsQuickBuildStatus. Three debugger paths call doSetStatus(...) directly and never pass through setStatus:

• BaseEditorActivity.kt:389 — doSetStatus(getString(string.debugger_started))
• BaseEditorActivity.kt:449 — doSetStatus(getString(string.debugger_starting))
• BaseEditorActivity.kt:453 — doSetStatus(getString(string.debugger_starting_failed))

Scenario: a Quick Build lands, so showQuickBuildStatus sets ownsQuickBuildStatus = true (ProjectHandlerActivity.kt:465) and the bar reads "reloaded to generation N in 2.1s". The user starts the debugger, which writes "Debugger starting" via doSetStatus — ownership is still held, because only setStatus clears it. The next session emission then hits one of two branches at ProjectHandlerActivity.kt:462 / :470:

• a passive refresh (Show(quick_build_status_ready, onlyIfOwned = true)) passes the ownsQuickBuildStatus guard and overwrites the debugger's line, or
• SessionStopped -> Clear passes the same guard and blanks it (setStatus("")).

Either way the debugger's status is lost to a writer that does not own the bar. Same class of bug as the StartFailed/upToDateUpdate replays from the earlier round, reached through a different door.

Two fixes, either is fine: route those three debugger calls through setStatus, or move ownsQuickBuildStatus = false into doSetStatus so the flag genuinely tracks "whoever writes the bar" rather than "whoever writes the bar via this one overload".

[jatezzz]

@fryanpan low (debug + CodeOnTheGo.qbbench only) — the autostart latch is armed before the build is known to have started, and a stuck latch silently eats the user's next install.

standardBuildStarted stamps standardBuildStartMs here, then runQuickBuild is called on the next line. But runQuickBuild can return without ever publishing a state (BuildViewModel.kt:81-90):

val current = _buildState.value
if (current is BuildState.InProgress) {
    log.warn("Build is already in progress. Ignoring new request.")
    onTerminalState?.invoke(BuildState.Error("A build is already in progress."))
    return
}

_buildState is untouched on that path, so onBuildStateChanged never fires, and fireAutostartStandardBuild passes no onTerminalState, so nothing clears the stamp. The latch stays armed indefinitely.

The next build to reach a terminal state is the user's, and onBuildStateChanged (ProjectHandlerActivity.kt:501) hands it straight to the armed latch:

• standardBuildEnded(isTerminal = true, isSuccess = true) finds a non-null startMs and returns true
• at :534, if (!suppressInstall) installApk(state) — so installApk is skipped

That is a successful build that ends with no install and no message, which is the same user-visible failure the reArmInstall net was added to prevent — and here the net cannot help, because the state was consumed as a deliberate suppression rather than a dropped one.

The window is real: autostart fires from project-open, and a session prebuild or a prior run can still hold InProgress. Two options: arm the latch from the InProgress transition rather than before the call, or pass an onTerminalState that clears the stamp so a refused runQuickBuild disarms it.

[jatezzz]

@fryanpan low — stageDaemon deletes and re-extracts the daemon jars on every provisioning run, including REBASELINE, which runs against a live session.

stageDaemon unconditionally wipes daemonDir and re-extracts:

if (daemonDir.exists()) {
    daemonDir.deleteRecursively()
}
Environment.mkdirIfNotExists(daemonDir)
val count = extractDaemonZip(context.assets.open(ASSET_DAEMON_ZIP).buffered(), daemonDir)

GradleQuickBuildProvisioner.kt:331 reaches it from runProxyAppBuild, which serves PROVISION, PREBUILD and REBASELINE. The first two run before a session is usable, so wiping is harmless there. A rebaseline is different: it happens while a session is live, which means the compile daemon process may still be running off daemon/*.jar.

The JVM keeps jar entries open lazily, so a class the daemon has not loaded yet is read from disk at first use. If that read lands in the window between deleteRecursively() and extractDaemonZip finishing, the daemon gets a NoClassDefFoundError / ZipException out of a jar that vanished under it, rather than the clean provisioning failure the caller is prepared to handle — and the error surfaces from the daemon side, where it is much harder to attribute.

Note the neighbouring stageRuntimeAar overwrites its target in place rather than deleting first, so it does not have this window — the delete is what makes stageDaemon different.

I could not establish from this diff whether a daemon is guaranteed dead by the time rebuildProxyApp reaches stage(); the isBuildInProgress guard just above covers the Gradle slot, not the compile daemon. If the daemon is guaranteed stopped, that invariant deserves a one-line comment here, because nothing at this call site states it. If it isn't, extract to a fresh directory and swap, so a running process never sees a missing jar.