Add find-in-conversation message search#2079
Merged
Merged
Conversation
Adds per-conversation content search to ChannelMessageList and UserMessageList (issue meshtastic#2045), matching Android's find-in-conversation UX: a search bar with live match count, current-match index, and previous/next navigation, scoped to the open conversation and covering the whole message history (not just the loaded window). Search runs on a background @Modelactor (MessageSearchActor) so the case/diacritic-insensitive substring scan never blocks the main thread, returning Sendable MessageSearchMatch value structs. Results are debounced (250ms), fetched with propertiesToFetch to avoid hydrating message text/relationships, and skipped entirely when the search bar is closed. Navigation expands the windowed list (via cheap fetchCount newer-counts) until the match is loaded, then scrolls to and highlights it (reusing the existing messageToHighlight binding, which now also gives reply-jumps a visible highlight). Adds MessageSearchTests (6 tests) covering substring/case/diacritic matching, conversation scoping, emoji exclusion, ordering, and newer-count; verified on iOS 18.2 and iOS 26.3. Fixes meshtastic#2045
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds per-conversation message-content search for channels and direct messages, with SwiftData queries, debounced navigation UI, match highlighting, tests, documentation, and Xcode project integration. ChangesConversation Search
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MessageSearchBar
participant ChannelMessageList
participant UserMessageList
participant MessageSearchActor
participant ModelContext
User->>MessageSearchBar: enter query
MessageSearchBar->>ChannelMessageList: update channel search state
MessageSearchBar->>UserMessageList: update direct-message search state
ChannelMessageList->>MessageSearchActor: request channel matches
UserMessageList->>MessageSearchActor: request direct-message matches
MessageSearchActor->>ModelContext: fetch matching messages
ModelContext-->>MessageSearchActor: return match cursors
MessageSearchActor-->>ChannelMessageList: return ordered channel matches
MessageSearchActor-->>UserMessageList: return ordered direct-message matches
ChannelMessageList-->>User: show count, scroll, and highlight
UserMessageList-->>User: show count, scroll, and highlight
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 3
🧹 Nitpick comments (3)
Meshtastic/Views/Messages/UserMessageList.swift (2)
661-673: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
directNewerCountruns a full-store count scan on the main thread.The substring
directMatchesscan was deliberately moved onto the backgroundMessageSearchActor, butensureLoadedstill callsMessageSearch.directNewerCount(in: context, …)against the main-actor@EnvironmentmodelContext. That issues up to fourfetchCountqueries synchronously on the main thread whenever the user navigates to a match outside the loaded window. On large conversations this can jank the UI at exactly the moment the user expects smooth scroll-to-match.Consider adding a
directNewerCountmethod toMessageSearchActorand awaiting it, keeping parity with the async search path. Note this only fires for off-window matches, so impact is occasional.🤖 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 `@Meshtastic/Views/Messages/UserMessageList.swift` around lines 661 - 673, Move the directNewerCount query out of the main-actor ensureLoaded flow by adding an asynchronous directNewerCount method to MessageSearchActor and invoking it with await. Update ensureLoaded to handle the actor result before adjusting messageLimit or calling loadMessages, while preserving the existing error logging and loaded-window behavior.
574-692: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the shared search/navigation flow
UserMessageListandChannelMessageListstill duplicate most of the search state machine. Only the match/count providers and a few conversation-specific strings differ; pulling the common debounce/navigation logic into a reusable helper would keep them from drifting.🤖 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 `@Meshtastic/Views/Messages/UserMessageList.swift` around lines 574 - 692, Extract the duplicated search state-machine logic from UserMessageList, including debouncedSearch, runSearch orchestration, focusMatch, next/previous navigation, and closeSearch, into a reusable helper shared with ChannelMessageList. Parameterize the helper with conversation-specific match/count providers and differing strings while preserving existing loading, stale-query, highlighting, and empty-result behavior in both views.Meshtastic/Views/Messages/ChannelMessageList.swift (1)
456-460: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSearch results go stale while new matching messages arrive.
.task(id: searchQuery)only re-runs when the query text changes. Unlikemessages(refreshed via.onChange(of: appState.unreadChannelMessages)at Line 417),searchMatches/matchCountaren't refreshed when new messages land in the channel while the search bar is open, so a newly-arrived matching message won't show up in the live count until the user retypes or reopens search.♻️ Proposed fix
.onChange(of: appState.unreadChannelMessages) { refreshIfNeeded() + if isSearching { Task { await runSearch() } } }🤖 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 `@Meshtastic/Views/Messages/ChannelMessageList.swift` around lines 456 - 460, Update the search-results refresh flow around debouncedSearch and the existing unreadChannelMessages change handler so active searches rerun when new channel messages arrive, not only when searchQuery changes. Preserve the current debouncing behavior and ensure searchMatches and matchCount are refreshed using the latest messages.
🤖 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 `@Meshtastic/Resources/docs/markdown/user/messages.md`:
- Line 111: Align the search behavior with the documentation by updating both
MessageSearch.channelMatches and channelNewerCount to exclude admin/system
messages using an admin == false predicate. Preserve existing channel filtering
and count behavior for non-admin messages.
In `@Meshtastic/Views/Messages/ChannelMessageList.swift`:
- Around line 574-587: The ensureLoaded function currently reloads only when the
required message count exceeds messageLimit. When the match is absent from
messages, always call loadMessages(markReadAfterLoad: false), while still
increasing messageLimit first when needed exceeds the current limit.
In `@Meshtastic/Views/Messages/ChannelMessageRow.swift`:
- Around line 173-178: Guard the delayed reset of messageToHighlight in
ChannelMessageRow so it only clears the highlight if the binding still
references the same messageId captured when the reply-jump timer was scheduled.
Preserve newer search highlights by skipping the reset when the value has
changed.
---
Nitpick comments:
In `@Meshtastic/Views/Messages/ChannelMessageList.swift`:
- Around line 456-460: Update the search-results refresh flow around
debouncedSearch and the existing unreadChannelMessages change handler so active
searches rerun when new channel messages arrive, not only when searchQuery
changes. Preserve the current debouncing behavior and ensure searchMatches and
matchCount are refreshed using the latest messages.
In `@Meshtastic/Views/Messages/UserMessageList.swift`:
- Around line 661-673: Move the directNewerCount query out of the main-actor
ensureLoaded flow by adding an asynchronous directNewerCount method to
MessageSearchActor and invoking it with await. Update ensureLoaded to handle the
actor result before adjusting messageLimit or calling loadMessages, while
preserving the existing error logging and loaded-window behavior.
- Around line 574-692: Extract the duplicated search state-machine logic from
UserMessageList, including debouncedSearch, runSearch orchestration, focusMatch,
next/previous navigation, and closeSearch, into a reusable helper shared with
ChannelMessageList. Parameterize the helper with conversation-specific
match/count providers and differing strings while preserving existing loading,
stale-query, highlighting, and empty-result behavior in both views.
🪄 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 Plus
Run ID: b14fcbab-0712-4d19-b2e9-7f77fd4bca25
📒 Files selected for processing (12)
Meshtastic.xcodeproj/project.pbxprojMeshtastic/Extensions/SwiftData/MessageSearch.swiftMeshtastic/Resources/docs/index.jsonMeshtastic/Resources/docs/markdown/user/messages.mdMeshtastic/Resources/docs/user/messages.htmlMeshtastic/Views/Messages/ChannelMessageList.swiftMeshtastic/Views/Messages/ChannelMessageRow.swiftMeshtastic/Views/Messages/MessageSearchBar.swiftMeshtastic/Views/Messages/UserMessageList.swiftMeshtastic/Views/Messages/UserMessageRow.swiftMeshtasticTests/MessageSearchTests.swiftdocs/user/messages.md
- ensureLoaded: reload the window whenever the search match isn't in the current messages array, not only when the window must grow — otherwise a match that should already be in range but isn't loaded left the highlight/scroll with nothing to target. - Reply-jump's delayed highlight clear now only resets messageToHighlight when it still points at that jump's message, so it can't wipe a newer search highlight set in the intervening second. - Correct the messages.md search note: channel search mirrors the channel list (which doesn't exclude admin), so drop the inaccurate blanket 'system/admin traffic excluded' claim.
# Conflicts: # Meshtastic.xcodeproj/project.pbxproj # Meshtastic/Resources/docs/index.json
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.
What changed?
Adds find in conversation — per-conversation message content search — to both
ChannelMessageListandUserMessageList, matching Android's UX (meshtastic/Meshtastic-Android#5373).MessageSearchhelper (SwiftData) + sharedMessageSearchBarview; search logic lives in same-file extensions to keep the view bodies focused.Why did it change?
Per meshtastic/design#75, there was no way to search message content on iOS —
.searchableonly filtered the contact/node list, and the conversation views had no search at all. Long conversations were hard to navigate.Per the issue, this uses an
NSPredicate/#Predicatesubstring match (localizedStandardContains) rather than an FTS5 virtual table — the simpler first-pass approach the issue explicitly allows. Historical messages are included (no reinstall required).Fixes #2045
Performance
Because the app already has performance-sensitive areas, the search path was built to stay off the hot path:
CONTAINSscan runs on a background@ModelActor(MessageSearchActor); onlySendableMessageSearchMatchvalue structs cross back to the main actor, so nothing blocks typing/scrolling regardless of conversation size.propertiesToFetch = [messageId, messageTimestamp]avoids hydrating message text + relationships for every hit.fetchCount"newer-count" queries (no object hydration) and only expands the window when jumping to an off-window match.How is this tested?
New
MessageSearchTests(6 tests): substring + case/diacritic-insensitivity, channel scoping + emoji exclusion, empty-query, channel newer-count, direct search spanning incoming+outgoing with user scoping + emoji exclusion, and direct newer-count. The tests assert exact match sets/counts.Run and passing on both iOS 18.2 and iOS 26.3 (cross-version check because SwiftData relationship-vs-nil predicates have OS-specific miscount/crash caveats — the leading concrete scalar term keeps this shape safe, and the exact-count assertions would catch a regression on either OS). Full
build-for-testingsucceeds under Swift 6 concurrency checking; SourceKit-LSP clean; reviewed by an internal Swift reviewer (concurrency model approved).Screenshots/Videos (when applicable)
UI: a find bar appears above the conversation with match position and up/down navigation; the current match is highlighted.
Checklist
docs/user/ordocs/developer/, and updated accordingly (see copilot-instructions.md for the view → doc page mapping). If no doc update is needed, add theskip-docs-checklabel.Summary by CodeRabbit