fix: propagate score_lines errors, fix compact response shape (CodeRabbit follow-up) - #196
Merged
Conversation
- score_lines now returns rusqlite::Result<Vec<ScoredLine>> instead of panicking via .expect() on every SQLite call (connection, table create, insert, query). handle_compact already returns Result and now propagates a scoring failure as an error; the PreCompact hook (which has no Result to propagate through) fails soft and skips output instead, consistent with its existing degrade-gracefully pattern for a missing/unreadable transcript. - handle_compact's empty-entries early return was missing "query" from its response, unlike the full response -- clients relying on a consistent shape could break on the empty-input path. Added it.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughCompact scoring now returns SQLite errors instead of panicking, and callers handle failures explicitly. Empty compaction responses now retain the input query, with tests updated for both behaviors. ChangesCompact scoring and response handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
3 tasks
This was referenced Jul 21, 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
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.
Summary
Follow-up addressing 2 CodeRabbit findings from the PR #193 review that were on
src/compact.rs/src/memory/mcp.rs(unrelated to #193's actual scope — that code came from an earlier duplicate branch and was reconciled against the already-merged, already-fixed version from #192/#195; these 2 are new findings against that merged code).score_lines()used.expect()on every SQLite call (connection, table create, insert, query) and would panic on any SQLite failure, even thoughhandle_compact(its only non-hook caller) already returns aResult. Now returnsrusqlite::Result<Vec<ScoredLine>>and propagates. The PreCompact hook caller has noResultto propagate through, so it fails soft (skips output) on error, consistent with its existing degrade-gracefully pattern for a missing/unreadable transcript.handle_compact's empty-entries early-return response ({"lines": [], "kept": 0, "total": 0}) was missing the"query"field the full response includes — a client relying on a consistent response shape could break on the empty-input path. Added it.Test plan
cargo test --workspace— all passing, new regression test for the response-shape fixcargo clippy --workspace --all-targets -- -D warnings -A unsafe_code -A clippy::pedantic— cleancargo fmt --check— cleanSummary by CodeRabbit