ADFA-4723 | Add ProjectSearchExtension API for plugin result sections#1528
Open
jatezzz wants to merge 6 commits into
Open
ADFA-4723 | Add ProjectSearchExtension API for plugin result sections#1528jatezzz wants to merge 6 commits into
jatezzz wants to merge 6 commits into
Conversation
![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.
jatezzz
requested review from
a team,
Daniel-ADFA,
dara-abijo-adfa and
itsaky-adfa
July 15, 2026 21:13
Contributor
📝 Walkthrough
WalkthroughChangesProject search integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ProjectHandlerActivity
participant PluginManager
participant ProjectSearchExtension
participant EditorViewModel
participant SearchResultFragment
participant SearchListAdapter
User->>ProjectHandlerActivity: start project search
ProjectHandlerActivity->>ProjectHandlerActivity: run recursive search
ProjectHandlerActivity->>PluginManager: get search extensions
ProjectHandlerActivity->>ProjectSearchExtension: searchProject(request)
ProjectSearchExtension-->>ProjectHandlerActivity: plugin sections
ProjectHandlerActivity->>EditorViewModel: publish combined sections
EditorViewModel-->>SearchResultFragment: emit searchResultSections
SearchResultFragment->>SearchListAdapter: render grouped results
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
app/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.kt (1)
49-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd KDoc to
SearchResultSection.This is a new public nested type consumed across three files (
SearchListAdapter,SearchResultFragment,ProjectHandlerActivity). A short doc clarifying thattitle == nullrenders as an untitled/legacy group and thatresultsgroups matches per file would help future maintainers.As per coding guidelines, "Public classes and functions, and non-obvious logic, require KDoc/Javadoc documenting contracts, rationale, threading, nullability, side effects, and units."
🤖 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 `@app/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.kt` around lines 49 - 53, Document the public nested data class SearchResultSection with concise KDoc describing that title being null represents an untitled or legacy group, and that results groups search matches by file.Source: Coding guidelines
plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt (1)
13-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the threading contract for
searchProject.The interface KDoc explains intent but doesn't state which thread invokes
searchProject, or whether plugins may block before returning the future. GivenProjectHandlerActivitycalls this synchronously per-plugin, clarifying this in the contract will prevent third-party plugins from doing blocking work on the wrong thread.As per coding guidelines, "Public classes and functions, and non-obvious logic, require KDoc/Javadoc documenting contracts, rationale, threading, nullability, side effects, and units."
🤖 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 `@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt` around lines 13 - 15, Update the KDoc for ProjectSearchExtension.searchProject to document that ProjectHandlerActivity invokes it synchronously on the calling thread and that plugins must not perform blocking work before returning the CompletableFuture; direct asynchronous or blocking work to an appropriate background executor.Source: Coding guidelines
app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt (1)
933-938: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd test coverage for the new plugin-aggregation logic.
requestPluginSearchSectionsand its conversion helpers are new non-UI business logic with non-trivial async/error-handling behavior (per-plugin exception isolation, future aggregation, section merging). No accompanying tests were included in this review's scope.As per coding guidelines, "New or changed non-UI code should achieve at least 50% line and branch coverage, with JaCoCo report numbers cited." Want me to draft a test skeleton (mocking
PluginManager/ProjectSearchExtension) covering the timeout/staleness scenarios above?🤖 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 `@app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt` around lines 933 - 938, add unit tests for requestPluginSearchSections and its conversion helpers, covering per-plugin exception isolation, asynchronous future aggregation, section merging, and timeout/staleness behavior. Mock PluginManager and ProjectSearchExtension as needed, and include JaCoCo line and branch coverage results meeting the required 50% threshold.Source: Coding guidelines
app/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.kt (1)
38-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating the two constructors into a single sections-based model.
The map-based constructor computes
rowsunconditionally, while the sections-based constructor discards that (emptyMap()) and stores real data in a second nullable fieldsectionRows, withvisibleRowspicking whichever is non-null. The only reason the map constructor still exists appears to beSearchResultFragment.onCreateAdapter()'s no-op adapter. That call site could just passemptyList<SearchResultSection>(), letting you drop the map constructor,rows, and thesectionRowsduality in favor of one constructor and one backing field.♻️ Sketch of a simplified single-constructor design
class SearchListAdapter( - results: Map<File, List<SearchResult>?>, + sections: List<SearchResultSection>, private val onFileClick: (File) -> Unit, private val onMatchClick: (SearchResult) -> Unit, - keys: List<File> ) : Adapter<ViewHolder>() { - private val rows: List<Row> = buildRows(listOf(SearchResultSection(null, results.filterValues { it != null }.mapValues { it.value.orEmpty() })), keys) + private val rows: List<Row> = buildRows(sections, keys = null) constructor( results: Map<File, List<SearchResult>?>, onFileClick: (File) -> Unit, onMatchClick: (SearchResult) -> Unit - ) : this(results, onFileClick, onMatchClick, results.keys.toList()) - - constructor( - sections: List<SearchResultSection>, - onFileClick: (File) -> Unit, - onMatchClick: (SearchResult) -> Unit - ) : this(emptyMap(), onFileClick, onMatchClick, emptyList()) { - sectionRows = buildRows(sections, null) - } - - private var sectionRows: List<Row>? = null + ) : this( + listOf(SearchResultSection(null, results.filterValues { it != null }.mapValues { it.value.orEmpty() })), + onFileClick, + onMatchClick + )Then replace
visibleRowsusages withrowsdirectly, and updateSearchResultFragment.onCreateAdapter()to callSearchListAdapter(emptyList(), noOp, noOp).🤖 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 `@app/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.kt` around lines 38 - 61, Consolidate SearchListAdapter around a single sections-based constructor: remove the map-based constructor, the eagerly computed rows property, and nullable sectionRows field, retaining one non-null rows backing field initialized from buildRows(sections, null). Update visibleRows usages to read rows directly, and change SearchResultFragment.onCreateAdapter() to pass emptyList<SearchResultSection>() with its existing no-op callbacks.
🤖 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/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt`:
- Around line 933-988: Update requestPluginSearchSections to bound plugin result
collection with a timeout so a stuck search cannot delay completion
indefinitely, and add a generation or active-search check in the UI callback
before calling editorViewModel.onSearchResultSectionsReady. Ensure only results
from the currently active/latest search are published, while timed-out plugin
futures are excluded or treated as empty.
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt`:
- Around line 17-37: ### Summary: Add KDoc to the public project-search contract
types and document coordinate indexing. Add KDoc for ProjectSearchRequest,
ProjectSearchSection, and ProjectSearchResult, including each property’s
meaning, nullability, units, and relevant contract behavior; explicitly state
whether startLine/startColumn/endLine/endColumn are 0- or 1-based and ensure the
documented convention matches the values consumed by Position/Range. Document
the request fields, section grouping, result preview/match semantics, and
nullable score without changing the data model.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt`:
- Around line 933-938: add unit tests for requestPluginSearchSections and its
conversion helpers, covering per-plugin exception isolation, asynchronous future
aggregation, section merging, and timeout/staleness behavior. Mock PluginManager
and ProjectSearchExtension as needed, and include JaCoCo line and branch
coverage results meeting the required 50% threshold.
In `@app/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.kt`:
- Around line 38-61: Consolidate SearchListAdapter around a single
sections-based constructor: remove the map-based constructor, the eagerly
computed rows property, and nullable sectionRows field, retaining one non-null
rows backing field initialized from buildRows(sections, null). Update
visibleRows usages to read rows directly, and change
SearchResultFragment.onCreateAdapter() to pass emptyList<SearchResultSection>()
with its existing no-op callbacks.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.kt`:
- Around line 49-53: Document the public nested data class SearchResultSection
with concise KDoc describing that title being null represents an untitled or
legacy group, and that results groups search matches by file.
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt`:
- Around line 13-15: Update the KDoc for ProjectSearchExtension.searchProject to
document that ProjectHandlerActivity invokes it synchronously on the calling
thread and that plugins must not perform blocking work before returning the
CompletableFuture; direct asynchronous or blocking work to an appropriate
background executor.
🪄 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: 593cf9e7-85d4-4216-aba4-668d7490f728
📒 Files selected for processing (7)
app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/SearchResultFragment.ktapp/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.ktapp/src/main/res/layout/layout_search_result_section.xmlplugin-api/api/plugin-api.apiplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughProject search now supports asynchronous plugin-provided sections, combines them with filesystem matches, and renders grouped results. Debug-client initialization now catches ChangesProject Search Integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ProjectHandlerActivity
participant ProjectSearchExtension
participant EditorViewModel
participant SearchResultFragment
User->>ProjectHandlerActivity: start project search
ProjectHandlerActivity->>ProjectSearchExtension: searchProject(request)
ProjectSearchExtension-->>ProjectHandlerActivity: return sections
ProjectHandlerActivity->>EditorViewModel: publish combined sections
EditorViewModel-->>SearchResultFragment: emit searchResultSections
SearchResultFragment->>SearchResultFragment: render grouped results
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt (1)
17-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd KDoc for public data classes and their properties.
As per coding guidelines: "Public classes and functions, and non-obvious logic, require KDoc/Javadoc documenting contracts, rationale, threading, nullability, side effects, and units."
Please documentProjectSearchRequest,ProjectSearchSection, andProjectSearchResultto clarify their contracts for plugin developers.🤖 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 `@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt` around lines 17 - 37, Add KDoc to the public data classes ProjectSearchRequest, ProjectSearchSection, and ProjectSearchResult, documenting each class’s purpose and the meaning, nullability, and units of every property, including line and column indexing conventions and the optional score contract.Source: Coding guidelines
app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt (1)
1094-1101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a safe call
?.letto avoid unnecessary lambda execution.
(result.cause as? SocketException?).letuses a standard.leton a nullable type, meaning the lambda executes even when the cause is not aSocketException(witherrasnull). Remove the inner?and use?.letso the block only runs for actualSocketExceptions.♻️ Proposed fix
- ?: (result.cause as? SocketException?).let { err -> - val msg = err?.message ?: "" + ?: (result.cause as? SocketException)?.let { err -> + val msg = err.message ?: "" when { msg.contains("EPERM") -> getString(string.debugger_error_errno_eperm) msg.contains("ECONNREFUSED") -> getString(string.debugger_error_errno_econnrefused)🤖 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 `@app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt` around lines 1094 - 1101, Update the nullable SocketException handling in the result-cause expression to use a safe-call let, removing the redundant nullable type marker so the lambda executes only when the cause is a SocketException. Preserve the existing errno message matching and fallback behavior.
🤖 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/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt`:
- Around line 963-967: Replace Throwable with Exception in both
ProjectHandlerActivity catch blocks: the plugin invocation failure handling
around lines 963-967 and the debug client connection handling around lines
1073-1085. Preserve the existing logging, crash recording, and recovery behavior
while allowing fatal JVM errors to propagate.
- Around line 973-980: Update the future aggregation in the
CompletableFuture.allOf thenRun block to apply orEmpty() to each future’s getNow
result before flatMap, so Java plugins returning null are treated as empty lists
and the existing section conversion flow remains unchanged.
In `@app/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.kt`:
- Around line 90-104: Flatten nested search results into the top-level
SearchListAdapter rows to eliminate the inner RecyclerView. Add Row.Match,
include each group’s SearchResult entries in buildRows, support a match view
type and ChildVH binding via bindMatch, and update onBindViewHolder accordingly;
remove inner RecyclerView setup from bindGroup and delete the items view from
the group layout while preserving group and match click behavior.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt`:
- Around line 1094-1101: Update the nullable SocketException handling in the
result-cause expression to use a safe-call let, removing the redundant nullable
type marker so the lambda executes only when the cause is a SocketException.
Preserve the existing errno message matching and fallback behavior.
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt`:
- Around line 17-37: Add KDoc to the public data classes ProjectSearchRequest,
ProjectSearchSection, and ProjectSearchResult, documenting each class’s purpose
and the meaning, nullability, and units of every property, including line and
column indexing conventions and the optional score contract.
🪄 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: d899709d-5da3-4893-bbcb-19ccc0584ee5
📒 Files selected for processing (7)
app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/SearchResultFragment.ktapp/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.ktapp/src/main/res/layout/layout_search_result_section.xmlplugin-api/api/plugin-api.apiplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt
Collaborator
|
Looks like we still need |
hal-eisen-adfa
left a comment
hal-eisen-adfa
left a comment
Collaborator
There was a problem hiding this comment.
A few plugin-isolation concerns in the search fan-out, plus a now-dead state flow. Details inline.
Fan out to enabled plugins after built-in search and render their sections.
- Bound plugin search fan-out with a 10s timeout and drop results from superseded searches via a generation counter in EditorViewModel - Guard against Java plugins completing their future with null - Flatten SearchListAdapter's nested RecyclerView into Row.Match rows so match rows recycle instead of inflating all at once - Narrow catch(Throwable) to catch(Exception) per REVIEW.md - Document plugin-api search contract types; coordinates are 0-based, end-exclusive - Marshal plugin crash recording to the UI thread and drop the unread searchResults flow.
jatezzz
force-pushed
the
feat/ADFA-4723-project-search-extension
branch
from
July 16, 2026 18:15
06a2c5c to
756134d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.kt`:
- Around line 123-129: Update the exception handler in the search matching flow
to log the caught exception through the existing SLF4J logger, then remove the
ThreadUtils UI fallback that assigns match.match to text.text. Preserve the
synchronously assigned match.line content and retain the existing thread-safe
success behavior.
- Around line 177-180: Update the results iteration around section.results so
file groups are added only when their match list is non-empty. Filter or guard
the mapped matches before adding Row.Group(file), while preserving the existing
Row.Match(match) entries for non-empty lists.
- Line 102: Update the click listener in the adapter binding logic from
binding.root to binding.item so the clickable nested LinearLayout invokes
onFileClick(file) when the row is tapped.
In `@app/src/main/res/layout/layout_search_result_group.xml`:
- Around line 25-30: Mark the ImageView with id icon as unavailable to
accessibility by adding the appropriate XML accessibility attribute, preserving
the existing layout and visual behavior.
🪄 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: 6390f2fc-7e22-42fd-bc5d-1d22fe90c6f8
📒 Files selected for processing (6)
app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.ktapp/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.ktapp/src/main/res/layout/layout_search_result_group.xmlapp/src/main/res/layout/layout_search_result_item.xmlplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt
- app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt (1)
1118-1118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant nullable type in safe cast.
In Kotlin, the safe cast operator
as?already returns a nullable type. Appending?to the target type is redundant and will cause a compiler warning.
app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt#L1118-L1118: remove?to useas? SocketExceptionapp/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt#L1126-L1126: remove?to useas? ErrnoExceptionfor both casts in this line🤖 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 `@app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt` at line 1118, Remove the redundant nullable markers from the safe casts in ProjectHandlerActivity: use as? SocketException at line 1118 and as? ErrnoException for both casts at line 1126, preserving the existing error-handling logic.
🤖 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/src/main/res/layout/layout_search_result_group.xml`:
- Around line 2-41: Remove the redundant RelativeLayout wrapper and make the
existing LinearLayout with id item the root view in
layout_search_result_group.xml. Preserve its current sizing, clickability,
focusability, foreground ripple, padding, and child views so binding.root
attaches directly to the clickable search-result surface.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt`:
- Line 1118: Remove the redundant nullable markers from the safe casts in
ProjectHandlerActivity: use as? SocketException at line 1118 and as?
ErrnoException for both casts at line 1126, preserving the existing
error-handling logic.
🪄 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: 3dd44250-2810-45df-9311-e00b91454410
📒 Files selected for processing (9)
app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/SearchResultFragment.ktapp/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.ktapp/src/main/res/layout/layout_search_result_group.xmlapp/src/main/res/layout/layout_search_result_item.xmlapp/src/main/res/layout/layout_search_result_section.xmlplugin-api/api/plugin-api.apiplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- app/src/main/res/layout/layout_search_result_item.xml
- app/src/main/res/layout/layout_search_result_section.xml
- plugin-api/api/plugin-api.api
- plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt
- app/src/main/java/com/itsaky/androidide/fragments/SearchResultFragment.kt
- Make the group row's LinearLayout the layout root so the click listener on binding.root receives taps (the clickable child was consuming them) and drop the redundant RelativeLayout wrapper - Log highlight failures via SLF4J and keep the plain-text preview instead of overwriting the line with the matched text - Skip files with empty match lists to avoid dangling group headers - Hide the decorative file icon from accessibility services
hal-eisen-adfa
left a comment
hal-eisen-adfa
left a comment
Collaborator
There was a problem hiding this comment.
Automated code review (xhigh effort) of the ProjectSearchExtension change. 9 inline findings below: 2 correctness (#1, #2), a retention leak (#3), a UX gap (#4), efficiency/simplification (#5, #6, #7), and a convention + docs nit (#9, #10). Numbering matches the review summary; #8 (long-line wrap) and #11 (highlight fallback text) were intentionally skipped.
Fan-out, adapter, and plugin-API fixes from the ProjectSearchExtension review: - Guard plugin results against null elements (mapNotNull) and wrap the collection block in a logging try/catch so a bad element no longer kills the whole fan-out silently. (#1) - buildRows buffers each section's rows and emits the header only when a group exists, skipping blank titles -- no more orphan/blank headers suppressing the empty state. (#2) - Move the fan-out to lifecycleScope so a slow plugin can't pin a destroyed Activity/ViewModel or post to a dead Activity. (#3) - Keep the search progress indicator up when the built-in search is empty but plugins are enabled, so a plugin-only match doesn't flash a terminal "no results" state. (#4) - SearchListAdapter is now a ListAdapter + DiffUtil reused across emissions; the two-phase publish diffs instead of reallocating, preserving scroll and already-run highlights. (#5) - Collapse the adapter constructors to one clicks ctor plus a thin map convenience ctor; drop the dead keys param and nullable-value scrub. (#6) - Replace the blocking .get(10s) with a non-blocking withTimeoutOrNull + await so no commonPool worker is tied up. (#7) - Trim pipe-split filter tokens at the source (endsWith(" kt") matched nothing in built-in search too); align ProjectSearchExtension KDoc with actual behavior and drop the non-ASCII em-dash. (#9, #10)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.kt (1)
129-132: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winNarrow the exception handling to catch specific crash types.
Based on learnings, prefer catching only the specific expected exceptions (such as
IllegalArgumentExceptionorIndexOutOfBoundsExceptionreported from the highlighter) instead of using a broadcatch (e: Exception)catch-all. This enforces fail-fast behavior for unexpected crashes during development.✨ Proposed fix
- } catch (e: Exception) { + } catch (e: IllegalArgumentException) { // The plain preview set above stays in place; only the highlight is lost. logger.warn("Failed to highlight search result preview", e) }🤖 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 `@app/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.kt` around lines 129 - 132, Update the highlighter error handling in the surrounding search preview binding logic to catch only the expected highlighting failures, such as IllegalArgumentException and IndexOutOfBoundsException, instead of broad Exception. Preserve the existing plain preview fallback and warning log for those specific failures while allowing unexpected exceptions to propagate.Source: Learnings
🤖 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/src/main/java/com/itsaky/androidide/fragments/SearchResultFragment.kt`:
- Around line 67-70: Update the asynchronous callback passed to
SearchListAdapter.submit in SearchResultFragment so it assigns isEmpty only when
the fragment remains added and _binding is non-null. Preserve the existing
adapter.itemCount == 0 calculation while guarding the view-dependent state
update against view destruction.
---
Outside diff comments:
In `@app/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.kt`:
- Around line 129-132: Update the highlighter error handling in the surrounding
search preview binding logic to catch only the expected highlighting failures,
such as IllegalArgumentException and IndexOutOfBoundsException, instead of broad
Exception. Preserve the existing plain preview fallback and warning log for
those specific failures while allowing unexpected exceptions to propagate.
🪄 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: 24506928-20b1-44e4-b91d-723991520052
📒 Files selected for processing (5)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/SearchResultFragment.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/extensions/ProjectSearchExtension.kt
- app/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.kt
hal-eisen-adfa
approved these changes
Jul 17, 2026
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
2 participants
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.
Description
Added the
ProjectSearchExtensionAPI to allow plugins to contribute custom sections to the project search. The IDE now triggers the built-in text search first, then asynchronously requests enabled plugins for extra results, aggregating and rendering them as distinct sections in the Search Results tab.Details
ProjectSearchExtension,ProjectSearchRequest, andProjectSearchSectioninterfaces/data classes to theplugin-api.ProjectHandlerActivityto collect search results from plugins usingCompletableFuture.SearchListAdapterto support a grouped UI with section headers (layout_search_result_section.xml).EditorViewModelto track and emitSearchResultSectionstates.SearchResultFragmentto observe and pass sections to the adapter.document_5098118372101457803.mp4
Ticket
ADFA-4723
Observation
Plugin searches are handled safely; if a plugin crashes or fails during the search execution, the exception is caught, logged, and recorded in the plugin manager without disrupting the main built-in search results or the overall UI flow.