Store stability output of Android variants to their own folder#150
Store stability output of Android variants to their own folder#150matejdro wants to merge 4 commits intoskydoves:mainfrom
Conversation
WalkthroughThe Gradle plugin and IDEA collector were made variant/task-aware: Kotlin compile tasks are now configured per-task or per-variant, stability plugin output/input paths are mapped to task/variant-scoped Changes
Sequence Diagram(s)sequenceDiagram
participant Gradle as Gradle Plugin
participant Android as Android Variant System
participant Kotlin as KotlinCompile Task
participant Compiler as Kotlin Compiler Plugin
participant FS as Filesystem (build/stability)
participant IDEA as IDEA Collector
Gradle->>Android: query variants (if Android project)
alt Android project
Android->>Gradle: provide variant names
Gradle->>Kotlin: locate KotlinCompile tasks matching variant names
Kotlin->>Compiler: set OPTION_STABILITY_OUTPUT_DIR=build/stability/<variant>
Kotlin->>FS: mark build/stability/<variant> as optional output
else Non-Android
Gradle->>Kotlin: locate applicable KotlinCompile tasks
Kotlin->>Compiler: set OPTION_STABILITY_OUTPUT_DIR=build/stability[/test]
Kotlin->>FS: mark build/stability[/test] as optional output
end
Compiler->>FS: write stability-info.json (variant/task-scoped) and project-dependencies.txt (build/stability/project-dependencies.txt)
IDEA->>FS: enumerate build/stability and immediate subfolders
IDEA->>FS: read stability-info.json files and parse composable data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt`:
- Around line 291-295: The IDE tool window still only reads the flat path
"build/stability/stability-info.json" so after StabilityAnalyzerGradlePlugin.kt
started writing variant outputs into "stability/<variant>/stability-info.json"
the collector stops seeing Android results; update ComposableStabilityCollector
(class/method that scans build/stability) to either (a) enumerate the
build/stability directory and read stability-info.json from every subdirectory
(e.g., stability/<variant>/stability-info.json) or (b) fall back to the legacy
root file if present—locate the scan logic around the method that currently
constructs the path "build/stability/stability-info.json" and change it to walk
child directories or attempt both locations so both legacy and variant outputs
are discovered.
- Around line 129-135: The KotlinCompile task filters currently hardcode false
in the calls to isKotlinTaskApplicable, causing test compile tasks to be treated
as non-writing when stabilityValidation.includeTests is enabled; change those
calls so they pass the actual includeTests flag (the same boolean used for
stabilityValidation) instead of false in the
target.tasks.withType(KotlinCompile::class.java).named {
isKotlinTaskApplicable(it, false) } blocks (both the block that sets
outputs.dir(stabilityDir).optional(true) and the similar block at the later
location), so isKotlinTaskApplicable receives the real includeTests value and
test compile tasks correctly declare their build/stability outputs.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c30514d1-dc51-40cc-8ad8-c5204ecc728a
📒 Files selected for processing (1)
stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt
| val stabilityFolderName = if (variant.isBlank()) { | ||
| "stability" | ||
| } else { | ||
| "stability/$variant" | ||
| } |
There was a problem hiding this comment.
Update the IDEA collector before moving Android outputs.
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/toolwindow/ComposableStabilityCollector.kt lines 60-65 still only scan build/stability/stability-info.json. After this change, Android builds emit build/stability/<variant>/stability-info.json, so the tool window will stop discovering fresh Android results unless it also walks the variant subdirectories or you keep a compatibility file in the root folder.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt`
around lines 291 - 295, The IDE tool window still only reads the flat path
"build/stability/stability-info.json" so after StabilityAnalyzerGradlePlugin.kt
started writing variant outputs into "stability/<variant>/stability-info.json"
the collector stops seeing Android results; update ComposableStabilityCollector
(class/method that scans build/stability) to either (a) enumerate the
build/stability directory and read stability-info.json from every subdirectory
(e.g., stability/<variant>/stability-info.json) or (b) fall back to the legacy
root file if present—locate the scan logic around the method that currently
constructs the path "build/stability/stability-info.json" and change it to walk
child directories or attempt both locations so both legacy and variant outputs
are discovered.
✅ Addressed in commit b61d18a
There was a problem hiding this comment.
Hm that's a good point, it would break IDEA plugin. @skydoves how should we handle this? Would we add a variant property to the idea plugin?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
What I think we could do is, have Intellij plugin scan the stability folder + all the subfolders for the stability-info.json. That way it detects the json file regardless of the variant.
There was a problem hiding this comment.
I have added check-all-folders, however it is untested. I'm not sure how can I build the IntelliJ plugin, it does not appear to be documented. Or did I miss it?
matejdro
force-pushed
the
android-variants-separate-folder
branch
from
8611989 to
b61d18a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/toolwindow/ComposableStabilityCollector.kt`:
- Around line 125-126: In ComposableStabilityCollector.kt, the catch (e:
Exception) that swallows parse failures for stability-info.json should log the
failing folder path and exception instead of ignoring it; update the catch to
call the module/class logger (or Logger.getInstance(...).error) including the
folder or file path being parsed and e (stacktrace/message) so users can
distinguish "no output" from "failed to read/generated" when stability-info.json
parsing fails.
- Around line 63-64: The collector currently flattens variantFolders +
rootStabilityFolder into allStabilityFolders and loses variant identity, causing
duplicate composables to be indistinguishable and StabilityStats to overcount;
update ComposableStabilityCollector to either (A) carry variant metadata with
each folder entry (e.g., change the collection element to a data class or pair
like (variantName, folder) and propagate that through the map/reduce that builds
StabilityStats and the UI model) or (B) deduplicate exact composable entries
before computing the summary (compare by canonical path/signature and collapse
identical entries) and apply the same change to the other aggregation sites
around the blocks that build the stats and UI model so counts reflect unique
composables per variant or globally as intended.
- Around line 61-64: The loop currently builds allStabilityFolders from
rootStabilityFolder ("build/stability") only, so non-Android test outputs
written to "build/stabilityTest/stability-info.json" are missed; update the
collector to also consider "build/stabilityTest" (e.g., create a
rootStabilityTest = File(contentRoot.path, "build/stabilityTest") and include
its directory plus its variant children into allStabilityFolders), and only
add/scan that folder when the includeTests flag/property is enabled; keep
references to rootStabilityFolder, rootStabilityTest, variantFolders,
allStabilityFolders, and the existing folder loop so the new folder is treated
the same as the original.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b3523a1-aec5-4bfc-aa92-01021269f4ba
📒 Files selected for processing (2)
compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/toolwindow/ComposableStabilityCollector.ktstability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt
| val allStabilityFolders = variantFolders + rootStabilityFolder | ||
| for (folder in allStabilityFolders) { |
There was a problem hiding this comment.
Preserve variant identity or dedupe before building stats.
The collector now merges debug, release, *UnitTest, etc. into one flat list but throws away the folder/variant name. Shared composables will show up as indistinguishable duplicates, and StabilityStats will overcount them. Please either carry the variant through the model/UI or collapse exact duplicates before computing the summary.
Also applies to: 110-123, 133-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@compose-stability-analyzer-idea/src/main/kotlin/com/skydoves/compose/stability/idea/toolwindow/ComposableStabilityCollector.kt`
around lines 63 - 64, The collector currently flattens variantFolders +
rootStabilityFolder into allStabilityFolders and loses variant identity, causing
duplicate composables to be indistinguishable and StabilityStats to overcount;
update ComposableStabilityCollector to either (A) carry variant metadata with
each folder entry (e.g., change the collection element to a data class or pair
like (variantName, folder) and propagate that through the map/reduce that builds
StabilityStats and the UI model) or (B) deduplicate exact composable entries
before computing the summary (compare by canonical path/signature and collapse
identical entries) and apply the same change to the other aggregation sites
around the blocks that build the stats and UI model so counts reflect unique
composables per variant or globally as intended.
There was a problem hiding this comment.
That is a good point, but I feel extra UI is out of scope for this PR. What do you think @skydoves?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt`:
- Around line 87-92: The code uses stabilityInputFiles.setFrom(...) twice which
causes the second call to overwrite the first; change the second invocation to
stabilityInputFiles.from(target.layout.buildDirectory.file("stability/test/stability-info.json"))
so the test file is appended instead of replacing the initial
stability/stability-info.json entry, ensuring stabilityInputFiles contains both
files.
- Around line 175-187: The stabilityInputFiles property is being overwritten by
repeated setFrom() calls so only the last file (AndroidTest) is kept; update the
code that sets stabilityInputFiles (references: stabilityInputFiles and
variant.name in StabilityAnalyzerGradlePlugin.kt) to aggregate all three files
instead of replacing them—use a single setFrom(...) with a collection of files
or call from(...) / addAll(...) for each file (e.g., gather the three
target.layout.buildDirectory.file(...) results into a list and pass that to
stabilityInputFiles.setFrom or call stabilityInputFiles.from(...) for each) so
main variant, UnitTest and AndroidTest files are all included.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9d43cb7f-227f-42bc-bdd4-4819296aee53
📒 Files selected for processing (1)
stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt
| stabilityInputFiles.setFrom( | ||
| target.layout.buildDirectory.file("stability/stability-info.json"), | ||
| ) | ||
| stabilityInputFiles.setFrom( | ||
| target.layout.buildDirectory.file("stability/test/stability-info.json"), | ||
| ) |
There was a problem hiding this comment.
Bug: Second setFrom() overwrites the first input file.
setFrom() replaces the entire file collection. The call at line 90 discards the file added at line 87, so the dump task will only read from stability/test/stability-info.json. Use from() for the second call to append instead.
🐛 Proposed fix
stabilityInputFiles.setFrom(
target.layout.buildDirectory.file("stability/stability-info.json"),
)
- stabilityInputFiles.setFrom(
+ stabilityInputFiles.from(
target.layout.buildDirectory.file("stability/test/stability-info.json"),
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt`
around lines 87 - 92, The code uses stabilityInputFiles.setFrom(...) twice which
causes the second call to overwrite the first; change the second invocation to
stabilityInputFiles.from(target.layout.buildDirectory.file("stability/test/stability-info.json"))
so the test file is appended instead of replacing the initial
stability/stability-info.json entry, ensuring stabilityInputFiles contains both
files.
| stabilityInputFiles.setFrom( | ||
| target.layout.buildDirectory.file("stability/stability-info.json"), | ||
| target.layout.buildDirectory.file("stability/${variant.name}/stability-info.json"), | ||
| ) | ||
| stabilityInputFiles.setFrom( | ||
| target.layout.buildDirectory.file( | ||
| "stability/${variant.name}UnitTest/stability-info.json", | ||
| ), | ||
| ) | ||
| stabilityInputFiles.setFrom( | ||
| target.layout.buildDirectory.file( | ||
| "stability/${variant.name}AndroidTest/stability-info.json", | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Bug: Repeated setFrom() calls overwrite previous input files.
Same issue as the non-Android path—each setFrom() replaces the collection, so only the AndroidTest file will be read. The main variant and UnitTest files are discarded.
🐛 Proposed fix
stabilityInputFiles.setFrom(
target.layout.buildDirectory.file("stability/${variant.name}/stability-info.json"),
)
- stabilityInputFiles.setFrom(
+ stabilityInputFiles.from(
target.layout.buildDirectory.file(
"stability/${variant.name}UnitTest/stability-info.json",
),
)
- stabilityInputFiles.setFrom(
+ stabilityInputFiles.from(
target.layout.buildDirectory.file(
"stability/${variant.name}AndroidTest/stability-info.json",
),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt`
around lines 175 - 187, The stabilityInputFiles property is being overwritten by
repeated setFrom() calls so only the last file (AndroidTest) is kept; update the
code that sets stabilityInputFiles (references: stabilityInputFiles and
variant.name in StabilityAnalyzerGradlePlugin.kt) to aggregate all three files
instead of replacing them—use a single setFrom(...) with a collection of files
or call from(...) / addAll(...) for each file (e.g., gather the three
target.layout.buildDirectory.file(...) results into a list and pass that to
stabilityInputFiles.setFrom or call stabilityInputFiles.from(...) for each) so
main variant, UnitTest and AndroidTest files are all included.
1 participant
🎯 Goal
A continuation of the #101
Up until now, every Kotlin task dropped its stability output into
build/stability. This was problematic with multi-variant runs:PR fixes this by changing plugin to store stability output into
build/stability/$VARIANT(such as/build/stability/debug)Summary by CodeRabbit