fix: search ranking, layout gaps, content truncation in BrainBar#133
fix: search ranking, layout gaps, content truncation in BrainBar#133
Conversation
…formatters 1. FTS5 ranking: Add f.rank to search query, ORDER BY f.rank instead of c.rowid ASC. Results now sorted by BM25 relevance score. Score exposed as positive number in results (negated from FTS5's negative convention). 2. Layout: Remove trailing empty │ line after last search result. The └─ closer now follows directly after the last result's tags/content line. Separator │ lines still appear between results, just not after the last. 3. Content truncation: Increase summary/snippet display from 72 to 150 chars so results are actually readable in the terminal. 67/67 tests green (5 new: 2 layout, 1 truncation, 2 ranking). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@coderabbitai review |
|
@greptileai review |
✅ Actions performedReview triggered.
|
📝 Walkthrough📝 Walkthrough🚥 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 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 |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@codex review |
|
Good! The search results are only consumed by Status: ✅ No backward compatibility issues Summary of Bug Review✅ APPROVED - No Critical Bugs FoundIssues Found:
Verified Correct:
Code Quality:
Recommendations:
The code changes are solid and well-tested. This is a clean bug fix PR with good test coverage.   |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@brain-bar/Sources/BrainBar/BrainDatabase.swift`:
- Line 237: The change to ORDER BY f.rank breaks unread delivery watermark
semantics because the code still advances the unread watermark using maxRowID
from the returned page; when unreadOnly is true and limit is applied this can
mark unseen rows as delivered. Modify the logic that advances the
delivery/unread watermark (the code using maxRowID in the unreadOnly path) so
that when unreadOnly is true you either (a) compute the watermark from the
contiguous highest row_id sequence (sorting the returned rows by row_id and
taking the largest contiguous row_id) rather than the max row_id of the
f.rank-ordered page, or (b) force the query/ordering to preserve row_id
contiguity for watermark calculation; ensure references to f.rank remain for
relevance sorting but do not use that ordering to compute the unread watermark
in advanceUnread/unread delivery code paths.
In `@brain-bar/Tests/BrainBarTests/FormattersTests.swift`:
- Around line 327-340: The test testSearchResultsSummaryTruncatesAt150 only
checks that the full 200-char input isn't present; update it to assert the exact
truncation behavior by locating the test function and the variables longSummary,
out, and contentLine, then extract the displayed summary from contentLine and
assert it equals the expected 150-character truncated string (149 "x" characters
plus the ellipsis character) or otherwise assert the displayed summary length is
150 and ends with the ellipsis; this ensures Formatters.formatSearchResults
actually truncates to 150 chars.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 56589442-8683-4cc8-a788-1bbf50b138cb
📒 Files selected for processing (4)
brain-bar/Sources/BrainBar/BrainDatabase.swiftbrain-bar/Sources/BrainBar/Formatters.swiftbrain-bar/Tests/BrainBarTests/DatabaseTests.swiftbrain-bar/Tests/BrainBarTests/FormattersTests.swift
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (3.11)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-18T00:12:08.774Z
Learnt from: EtanHey
Repo: EtanHey/brainlayer PR: 87
File: brain-bar/Sources/BrainBar/BrainBarServer.swift:118-129
Timestamp: 2026-03-18T00:12:08.774Z
Learning: In Swift files under brain-bar/Sources/BrainBar, enforce that when a critical dependency like the database is nil due to startup ordering (socket before DB), any tool handler that accesses the database must throw an explicit error (e.g., ToolError.noDatabase) instead of returning a default/empty value. Do not allow silent defaults (e.g., guard let db else { return ... }). Flag patterns that silently return defaults when db is nil, as this masks startup timing issues. This guidance applies broadly to similar Swift files in the BrainBar module, not just this one location.
Applied to files:
brain-bar/Sources/BrainBar/Formatters.swiftbrain-bar/Sources/BrainBar/BrainDatabase.swift
🔇 Additional comments (3)
brain-bar/Sources/BrainBar/Formatters.swift (1)
111-111: Formatting updates look correct and aligned with expected UX.The 150-char truncation and “no trailing separator after last result” logic are clean and match the intended output behavior.
Also applies to: 132-135
brain-bar/Tests/BrainBarTests/FormattersTests.swift (1)
300-323: Good coverage for trailing separator regression.These tests correctly guard against the extra trailing
│line in both single and multi-result outputs.brain-bar/Tests/BrainBarTests/DatabaseTests.swift (1)
318-337: Nice additions for ranking regression protection.These tests provide useful guardrails for both score population and relevance ordering behavior.
…n test Address CodeRabbit review findings: 1. CRITICAL: ORDER BY f.rank broke unread delivery watermark. When unreadOnly=true, maxRowID from rank-sorted results could skip rowids and mark unseen chunks as delivered. Fix: use rowid ASC for unread queries, f.rank for normal search. 2. MAJOR: Truncation test was weak — only checked 200-char string absent. Now asserts exact 149 chars + ellipsis (150 total) is present. 67/67 tests green. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27b6830301
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| JOIN chunks c ON c.id = f.chunk_id | ||
| WHERE \(conditions.joined(separator: " AND ")) | ||
| ORDER BY c.rowid ASC | ||
| ORDER BY f.rank |
There was a problem hiding this comment.
Ordering all searches by f.rank breaks the delivery watermark logic when unreadOnly is used: this function still updates last_delivered_seq from maxRowID after a limited fetch, but relevance ordering is not monotonic by rowid. In a batch where a high-rowid item ranks highly, markDelivered can advance past older unread rows that were never returned, causing later unread/ack flows to skip those memories. Preserve rowid-ordered paging for unread mode (or only advance through contiguous rowids) so unread items are not dropped.
Useful? React with 👍 / 👎.
1 participant
Summary
Follow-up to #132. Fixes 4 issues found during live testing:
ORDER BY c.rowid ASC→ORDER BY f.rank(BM25 relevance). Results now sorted by actual match quality instead of insertion order.f.rankto SELECT, exposed as positivescorefield. Scores were always 0.00 before.│line after last search result.└─now follows directly.Files Changed
BrainDatabase.swiftf.rankto SELECT,ORDER BY f.rank, expose scoreFormatters.swift│, increase maxLen to 150DatabaseTests.swiftFormattersTests.swiftTest plan
│gap before└─🤖 Generated with Claude Code
Note
Fix search ranking, layout gaps, and content truncation in BrainBar
BrainDatabase.searchto use FTS5 BM25 relevance (f.rank) instead ofrowidfor non-unread searches; each result now includes a positivescorefield derived from the rank.Formatters.formatSearchResultsfrom 72 to 150 characters and removes the trailing│separator line after the last result.Macroscope summarized 805af1b.
Summary by CodeRabbit
New Features
Bug Fixes
Tests