Aggregate hub classes and entry points from per-build analysis#8
Merged
Conversation
The cross-build PSI analysis tried to parse every source file across all included builds in one shot, which OOMs on large repos. Hub classes, entry points, and cross-build reports all silently returned empty. Now aggregates results from per-build ProjectSummary.analysis data that SymbolExtractor already computes per included build. This is the same path anti-patterns use (which always worked). Removed the monolithic analyzeCrossBuild() and its collectAllSourceDirs() helper. Hub classes are merged and ranked by dependent count across all builds. Entry points are classified from source set metadata.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 5 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaces cross-build source scanning and full analysis pipeline in ContextTask with aggregation of existing per-project AnalysisSummary results; entry-point discovery now derives from ProjectSummary.sourceSets symbols with heuristic TEST/MOCK labeling. Aggregated summary is passed to DashboardRenderer and CrossBuildRenderer for rendering. Changes
Sequence Diagram(s)sequenceDiagram
participant ContextTask as ContextTask
participant Summaries as ProjectSummaries
participant Aggregator as aggregateAnalysis()
participant Dashboard as DashboardRenderer
participant CrossBuild as CrossBuildRenderer
Note over ContextTask,Summaries: start aggregation
ContextTask->>Summaries: collect root + included-build ProjectSummary lists
ContextTask->>Aggregator: aggregateAnalysis(summaryList, includedBuildSummaries)
Aggregator->>Aggregator: merge hubs (top-N by dependentCount), dedupe findings & cycles
Aggregator-->>ContextTask: AnalysisSummary? (aggregatedSummary)
ContextTask->>Dashboard: render with aggregatedSummary
ContextTask->>CrossBuild: render cross-build pages with aggregatedSummary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
🧹 Nitpick comments (2)
src/main/kotlin/zone/clanker/gradle/srcx/task/ContextTask.kt (1)
252-256: Make hub ranking deterministic for equaldependentCount.Current ordering only uses
dependentCount; ties can produce unstable output across runs. Add a secondary key (e.g.,name) to reduce report churn.Proposed refinement
val allHubs = allAnalyses .flatMap { it.hubs } - .sortedByDescending { it.dependentCount } + .sortedWith( + compareByDescending<zone.clanker.gradle.srcx.model.HubClass> { it.dependentCount } + .thenBy { it.name }, + ) .take(HUB_LIMIT)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/zone/clanker/gradle/srcx/task/ContextTask.kt` around lines 252 - 256, The current hub ranking in ContextTask.kt that builds allHubs (the expression using allAnalyses.flatMap { it.hubs }.sortedByDescending { it.dependentCount }.take(HUB_LIMIT)) is non-deterministic for equal dependentCount; change the sort to a stable comparator that breaks ties by hub name (e.g., sort by dependentCount descending then by name ascending) so allHubs ordering is deterministic across runs while still honoring HUB_LIMIT.src/test/kotlin/zone/clanker/gradle/srcx/task/AggregateAnalysisTest.kt (1)
37-168: Good coverage on merge/dedupe, but add regression tests for limit + entry-point classification.Please add cases for (1)
HUB_LIMITtruncation at 30 and (2)buildEntryPointsFromSummariesclassifying APP/TEST/MOCK (including mock source-set naming). These are central to this refactor and currently unguarded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/zone/clanker/gradle/srcx/task/AggregateAnalysisTest.kt` around lines 37 - 168, Add two tests: one that verifies HUB_LIMIT truncation by creating >30 HubClass entries (use HubClass constructor) across summaries and asserting task.aggregateAnalysis(...).hubs.size == 30 and that they are the top 30 by dependent count; and another that exercises buildEntryPointsFromSummaries by providing summaries with entry-point metadata whose sourceSet names indicate APP, TEST and MOCK (include common mock naming like "testFixtures" or "mock" and explicit test source-set) and assert resulting EntryPoint classifications are APP/TEST/MOCK as expected. Reference aggregateAnalysis, HUB_LIMIT, HubClass, and buildEntryPointsFromSummaries when locating where to add these tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/zone/clanker/gradle/srcx/task/ContextTask.kt`:
- Around line 196-231: The current buildEntryPointsFromSummaries function drops
non-test/non-mock classes by returning null for their kind; update the mapping
so that when a symbol is neither isTest nor isMock you assign
EntryPointsRenderer.EntryKind.APP (instead of null) so all classes are emitted
as EntryPointsRenderer.ClassifiedEntry(name, symbol.packageName.value,
EntryKind.APP) and continue to deduplicate with distinctBy; refer to
buildEntryPointsFromSummaries, EntryPointsRenderer.EntryKind and
EntryPointsRenderer.ClassifiedEntry to locate and change the else branch.
---
Nitpick comments:
In `@src/main/kotlin/zone/clanker/gradle/srcx/task/ContextTask.kt`:
- Around line 252-256: The current hub ranking in ContextTask.kt that builds
allHubs (the expression using allAnalyses.flatMap { it.hubs }.sortedByDescending
{ it.dependentCount }.take(HUB_LIMIT)) is non-deterministic for equal
dependentCount; change the sort to a stable comparator that breaks ties by hub
name (e.g., sort by dependentCount descending then by name ascending) so allHubs
ordering is deterministic across runs while still honoring HUB_LIMIT.
In `@src/test/kotlin/zone/clanker/gradle/srcx/task/AggregateAnalysisTest.kt`:
- Around line 37-168: Add two tests: one that verifies HUB_LIMIT truncation by
creating >30 HubClass entries (use HubClass constructor) across summaries and
asserting task.aggregateAnalysis(...).hubs.size == 30 and that they are the top
30 by dependent count; and another that exercises buildEntryPointsFromSummaries
by providing summaries with entry-point metadata whose sourceSet names indicate
APP, TEST and MOCK (include common mock naming like "testFixtures" or "mock" and
explicit test source-set) and assert resulting EntryPoint classifications are
APP/TEST/MOCK as expected. Reference aggregateAnalysis, HUB_LIMIT, HubClass, and
buildEntryPointsFromSummaries when locating where to add these tests.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec941d85-64b7-4fcd-a6a0-4c4c3a7acfbf
📒 Files selected for processing (2)
src/main/kotlin/zone/clanker/gradle/srcx/task/ContextTask.ktsrc/test/kotlin/zone/clanker/gradle/srcx/task/AggregateAnalysisTest.kt
When PSI analysis runs out of memory, now logs: "srcx: Out of memory analyzing 'project'. Increase heap with org.gradle.jvmargs=-Xmx8g in gradle.properties" Other failures log the specific error message instead of silently returning null.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/zone/clanker/gradle/srcx/scan/SymbolExtractor.kt`:
- Around line 45-54: handleAnalysisFailure currently logs OutOfMemoryError but
callers use getOrNull() which swallows OOM; change behavior so OOM is rethrown
to fail fast: add a helper (e.g., finalizeAnalysisFailure(e: Throwable,
projectName: String): AnalysisSummary?) that calls handleAnalysisFailure and
then throws when e is an OutOfMemoryError, otherwise returns null, and replace
occurrences that do .onFailure { e -> handleAnalysisFailure(e, ...)
}.getOrNull() with .getOrElse { e -> finalizeAnalysisFailure(e, projectName) }
(or equivalent) so non-fatal exceptions still return null but OutOfMemoryError
is propagated.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d499ba35-f1e5-423e-9777-0084fb37b4b7
📒 Files selected for processing (1)
src/main/kotlin/zone/clanker/gradle/srcx/scan/SymbolExtractor.kt
…hub sort - APP entry points were dropped (mapped to null) — now classified as APP - OOM rethrows after logging hint instead of silently returning null - Hub ranking uses secondary sort by name for deterministic output
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.
Summary
analyzeCrossBuild()which tried to PSI-parse every source file across all included builds at once — this OOMed on large repos, causing hub classes, entry points, and cross-build reports to silently return emptyProjectSummary.analysisdata thatSymbolExtractoralready computes per included build (same path anti-patterns use, which always worked)Test plan
./gradlew buildpasses (all tests, detekt, ktlint, kover)srcx-contextgenerates hub classes (419 lines, was "No hub classes detected")Summary by CodeRabbit