-
Notifications
You must be signed in to change notification settings - Fork 550
fix(hld): improve session search to match UI display logic #808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
Update SearchSessionsByTitle to: - Search across title, summary, and query fields (matching UI fallback display) - Filter to only leaf sessions (no children) - Exclude archived, draft, and discarded sessions This ensures cmd+k session search in WUI finds sessions that display their query/summary text when no explicit title is set. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to dc300ca in 2 minutes and 19 seconds. Click for details.
- Reviewed
219lines of code in3files - Skipped
0files when reviewing. - Skipped posting
8draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. hld/api/openapi.yaml:519
- Draft comment:
Good update: The OpenAPI spec now clearly documents that the search will match across title, summary, and query fields and returns only leaf (non-archived, non-draft, non-discarded) sessions. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, describing what the OpenAPI spec now documents. It doesn't provide a suggestion, ask for confirmation, or point out a potential issue.
2. hld/store/sqlite.go:1787
- Draft comment:
In SearchSessionsByTitle, the SQL query appends a fixed 'LIMIT 20' clause regardless of the user-provided limit, and then the result slice is truncated in Go. Consider parameterizing the LIMIT directly in the SQL query to avoid over-fetching rows when a lower limit is requested. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. hld/store/sqlite.go:1795
- Draft comment:
Update the inline comment: It currently refers to 'substring matching anywhere in title' but now the search applies to title, summary, and query fields. Adjust the comment for consistency. - Reason this comment was not posted:
Comment was on unchanged code.
4. hld/store/sqlite.go:1810
- Draft comment:
The filtering for leaf sessions using the 'NOT EXISTS' clause is well implemented and ensures that parent sessions are excluded. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the implementation without offering any specific guidance or questions.
5. hld/store/sqlite_test.go:864
- Draft comment:
The test cases for SearchSessionsByTitle are comprehensive, covering case-insensitive search, exclusion of archived/draft/discarded sessions, filtering to leaf sessions, and matching across all three fields. Excellent coverage! - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only praises the comprehensiveness of the test cases without providing any actionable feedback or suggestions for improvement.
6. hld/store/sqlite_test.go:453
- Draft comment:
The 'GetSessionConversationWithParentChain' tests effectively validate the parent-child-grandchild conversation chain ordering. This ensures that full historical context is retrieved in the right order. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. hld/store/sqlite.go:0
- Draft comment:
Overall, the use of parameterized queries and error handling is robust. Consider refactoring repeated scanning code in methods like GetSession and ListSessions into helper functions to reduce duplication, though this is a low-priority improvement. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
8. hld/store/sqlite.go:1816
- Draft comment:
Typographical issue: There appears to be an extraneous backtick at the end of the SQL condition on line 1816 (AND status != 'discarded''`). This backtick should likely be removed to avoid potential SQL syntax errors. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The comment is incorrect. The backtick is part of a multi-line SQL string literal in Go that starts several lines above. Removing it would break the SQL query. The backtick is intentional and required for the code to work properly. Could there be a case where the backtick should be moved to a different line for better readability? Should the SQL query formatting be changed? No, the current formatting is clear and follows standard Go practices for multi-line string literals. The backtick placement is correct and conventional. The comment is incorrect and should be deleted. The backtick is a necessary part of the Go string literal syntax and removing it would break the code.
Workflow ID: wflow_KAOYMS2W7XB4efxw
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
K-Mistele
requested changes
Oct 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 problem(s) was I solving?
Users couldn't find sessions in the cmd+k command palette when searching for text that was actually displayed in the UI. The disconnect happened because:
session.title || session.summary || session.query(see CommandPaletteMenu.tsx:218 and formatting.ts:160)SearchSessionsByTitlewas only searching thetitlefieldAdditionally, the search was returning sessions that shouldn't be user-facing:
What user-facing changes did I ship?
Users can now find sessions in cmd+k search by typing any part of the text they see displayed in the UI:
How I implemented it
Updated
SearchSessionsByTitlein hld/store/sqlite.go:title LIKE ?) to multi-field search (title LIKE ? OR summary LIKE ? OR query LIKE ?)archived = 1), draft (status = 'draft'), and discarded (status = 'discarded') sessionsAdded comprehensive test coverage in hld/store/sqlite_test.go:
How to verify it
make check testpassesManual Testing
To verify this fix manually:
make wui-dev)Description for the changelog
fix(hld): Session search now searches across title, summary, and query fields to match UI display logic; filters to only active leaf sessions