ADFA-3641: Wire per-task tooltips for Gradle tasks#1519
Conversation
Each task row in the Run-Tasks dialog now shows its own tooltip instead of a
single generic one. The tag is derived from the task path
("gradle." + path.removePrefix(":")), matching the authored content keys
(":app:assembleDebug" -> "gradle.app:assembleDebug", ":clean" -> "gradle.clean").
- TooltipTag.gradleTaskTooltipTag(): pure path->tag mapping (unit-tested).
- RunTasksListAdapter: use the per-task tag instead of PROJECT_GRADLE_TASKS.
- ToolTipManager.getTooltip: honor its nullable contract - return null on a
missing tag so uncovered tasks show no popup instead of an "n/a" tooltip.
No fallback.
Tooltip content lives in the SoT documentation.db (loaded separately).
Verified end-to-end on the emulator: per-task Tier-1/2 tooltip + Tier-3 help
page, and graceful no-popup for tasks without authored content.
![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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 Walkthrough
WalkthroughGradle task tooltips now use tags derived from task paths. Tooltip lookup constructs results only for successful database queries and returns ChangesTooltip behavior updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RunTasksListAdapter
participant TooltipTag
participant ToolTipManager
participant SQLiteDatabase
RunTasksListAdapter->>TooltipTag: gradleTaskTooltipTag(task.path)
TooltipTag-->>RunTasksListAdapter: path-specific tag
RunTasksListAdapter->>ToolTipManager: request tooltip with tag
ToolTipManager->>SQLiteDatabase: query tooltip and buttons
SQLiteDatabase-->>ToolTipManager: result row or failure
ToolTipManager-->>RunTasksListAdapter: IDETooltipItem or null
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (2)
idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt (2)
90-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse narrow exception handling instead of a generic catch-all.
Based on learnings, in Kotlin files across the AndroidIDE project, prefer narrow exception handling that catches only the specific exception type reported in crashes instead of a broad catch-all, aligning with fail-fast behavior during development.
idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt#L90-L93: Replacecatch (e: Exception)with a specific exception such ascatch (e: SQLiteException).idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt#L134-L140: Replacecatch (e: Exception)with a specific exception such ascatch (e: SQLiteException).🤖 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 `@idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt` around lines 90 - 93, Replace the broad catch-all handlers in ToolTipManager’s version-resolution blocks with narrow SQLiteException handling, preserving the existing logging and "n/a" fallback. Apply this change at ToolTipManager.kt lines 90-93 and 134-140; ensure the required SQLiteException import is available.Source: Learnings
131-133: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSilence the swallowed exception warning.
The static analysis tool warns about a swallowed exception here. Since this is an expected control-flow path, you can rename the exception parameter to
_to suppress the warning and clarify your intent to ignore it.♻️ Proposed refactor
- } catch (e: NoTooltipFoundException) { + } catch (_: NoTooltipFoundException) {🤖 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 `@idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt` around lines 131 - 133, Rename the unused exception parameter in the NoTooltipFoundException catch clause within the tooltip lookup flow to “_”, preserving the existing logging and null return behavior.Source: Linters/SAST tools
🤖 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
`@idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt`:
- Around line 90-93: Replace the broad catch-all handlers in ToolTipManager’s
version-resolution blocks with narrow SQLiteException handling, preserving the
existing logging and "n/a" fallback. Apply this change at ToolTipManager.kt
lines 90-93 and 134-140; ensure the required SQLiteException import is
available.
- Around line 131-133: Rename the unused exception parameter in the
NoTooltipFoundException catch clause within the tooltip lookup flow to “_”,
preserving the existing logging and null return behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be0e7ba3-0119-413b-8720-a86c3d9d107d
📒 Files selected for processing (5)
app/src/main/java/com/itsaky/androidide/adapters/RunTasksListAdapter.ktidetooltips/build.gradle.ktsidetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.ktidetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.ktidetooltips/src/test/java/com/itsaky/androidide/idetooltips/GradleTaskTooltipTagTest.kt
…ps-to-gradle-tasks # Conflicts: # idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt
Stage now runs spotlessCheck in CI (ADFA-2522). Spotless' ratchet is file-level, so the two files this branch touches get reformatted whole-file (space -> tab). Whitespace-only; no logic change.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt (2)
77-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAddress the TODO by restricting debug code to debug builds.
You can use
BuildConfig.DEBUGto ensure the debug database switch is only evaluated in debug builds, fulfilling the requirement mentioned in the TODO.Do you want me to generate a solution that implements this requirement and open a new issue to track it?
🤖 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 `@idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt` around lines 77 - 84, Update the debug database logic in ToolTipManager around debugDatabaseTimestamp so it is evaluated and can switch dbPath only when BuildConfig.DEBUG is true; preserve the existing timestamp comparison and production database behavior for non-debug builds.
190-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a lifecycle-bound coroutine scope.
Creating an unmanaged
CoroutineScopeviolates the coding guideline to use lifecycle-bound coroutine scopes and avoid retaining screen contexts. This "fire and forget" coroutine can outlive itsContext, potentially leading to memory leaks or wasted work if the context is destroyed while the database query is suspended.Consider extracting the
Context.findActivity()helper fromcanShowPopupto access the activity'slifecycleScope(e.g.,(context.findActivity() as? ComponentActivity)?.lifecycleScope?.launch), or pass an appropriateCoroutineScopefrom the caller. As per coding guidelines, use lifecycle-bound coroutine scopes.🤖 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 `@idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt` at line 190, Replace the unmanaged CoroutineScope in the popup/database-query flow with a lifecycle-bound scope, using the activity obtained through the existing Context.findActivity helper and its lifecycleScope, or an appropriate caller-provided scope. Ensure the coroutine is launched only when a valid lifecycle owner is available and preserve the existing work otherwise.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.
Inline comments:
In
`@idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt`:
- Around line 134-140: Update both exception handlers in ToolTipManager.kt at
lines 134-140 and 90-93: replace broad Exception catches with the specific
expected exception type where possible, or rethrow CancellationException before
logging and handling other failures. Preserve the existing fallback behavior and
logging for non-cancellation errors at both sites.
---
Nitpick comments:
In
`@idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.kt`:
- Around line 77-84: Update the debug database logic in ToolTipManager around
debugDatabaseTimestamp so it is evaluated and can switch dbPath only when
BuildConfig.DEBUG is true; preserve the existing timestamp comparison and
production database behavior for non-debug builds.
- Line 190: Replace the unmanaged CoroutineScope in the popup/database-query
flow with a lifecycle-bound scope, using the activity obtained through the
existing Context.findActivity helper and its lifecycleScope, or an appropriate
caller-provided scope. Ensure the coroutine is launched only when a valid
lifecycle owner is available and preserve the existing work otherwise.
🪄 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
Run ID: 240aedaa-b974-418c-ad98-9707df0b1c8f
📒 Files selected for processing (2)
idetooltips/src/main/java/com/itsaky/androidide/idetooltips/ToolTipManager.ktidetooltips/src/test/java/com/itsaky/androidide/idetooltips/GradleTaskTooltipTagTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- idetooltips/src/test/java/com/itsaky/androidide/idetooltips/GradleTaskTooltipTagTest.kt
A broad catch(Exception) in the getTooltip suspend function swallowed coroutine CancellationException, silently returning null (a "no tooltip" result) on cancellation. Rethrow it before the fallback catch to keep structured concurrency intact.
2 participants
Summary
Wires per-task tooltips into the Run-Tasks dialog (ADFA-3641). Each Gradle task row now shows its own authored tooltip (Tier-1/2) with a Tier-3 help page, instead of one generic tooltip for every task.
Changes
TooltipTag.gradleTaskTooltipTag(path)— pure mapping:":app:assembleDebug" → "gradle.app:assembleDebug",":clean" → "gradle.clean". Unit-tested.RunTasksListAdapter— long-press uses the per-task tag instead ofPROJECT_GRADLE_TASKS.ToolTipManager.getTooltip— now honors itsIDETooltipItem?contract: returnsnullon a missing tag (its only caller,showTooltip, already null-checks), so tasks without authored content show no popup instead of an "n/a" tooltip. No fallback.idetooltips/build.gradle.kts— addstestImplementation(libs.tests.junit)for the new test.Content
Tooltip content lives in the source-of-truth
documentation.db(loaded separately, not in this repo). 280 Gradle-task tooltips authored + wired.Verification
GradleTaskTooltipTagTestpasses (:idetooltips:testV8DebugUnitTest).:buildEnvironment→ task-specific tooltip → See more (Tier-2 bullet detail) → Learn more (Tier-3 help page served from theContenttable). Uncovered task:artifactTransforms→ no popup; logcat confirmsgetTooltip … → No tooltip found → null, no "n/a".Coverage
Live diff (261 UI tasks vs 282 loaded tags): the 280 covered tasks work; 41 UI tasks are uncovered (the
ideaGradle plugin family, plus Gradle/AGP version drift) — all degrade gracefully to no-popup. None are CoGo-specific. Full breakdown posted to ADFA-3641 for the content team.Notes
.gitpointer file); pre-commit / CI will apply formatting.