Skip to content

fix: BrainBar write retry cap + search filter params (C1/H3)#91

Merged
EtanHey merged 1 commit intomainfrom
fix/brainbar-critical-bugs
Mar 18, 2026
Merged

fix: BrainBar write retry cap + search filter params (C1/H3)#91
EtanHey merged 1 commit intomainfrom
fix/brainbar-critical-bugs

Conversation

@EtanHey
Copy link
Copy Markdown
Owner

@EtanHey EtanHey commented Mar 18, 2026

Summary

  • C1 (CRITICAL): Reduced sendResponse EAGAIN retry cap from 50→10 (10ms max serial queue block). Extracted maxWriteRetries constant. This was the probable root cause of BrainBar socket death — one stalled socat client blocked ALL connections.
  • H3 (HIGH): brain_search filter params (project, tag, importance_min) were advertised in the MCP tool schema but completely ignored by handleBrainSearch() and BrainDatabase.search(). Now search() builds a dynamic WHERE clause and MCPRouter extracts + passes all 3 filters.
  • C2/H1: Already fixed in current code (bounds check + write loop). Added tests to verify.
  • CodeRabbit pre-commit review: 2 issues found and fixed (silent guard return, off-by-one in log message).

Test plan

  • 37 Swift tests pass (9 new: 5 DB filter, 2 E2E router, stalled client, overlong path)
  • 921 Python tests pass (0 failures from changes)
  • CodeRabbit pre-commit review passed (2 issues fixed)

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

Summary by CodeRabbit

  • New Features

    • Added search filtering capabilities: filter results by project, tag, and minimum importance level.
    • Added support for tagging content when storing information.
  • Improvements

    • Enhanced connection stability with improved handling and logging for stalled clients.

…iew mining

C1: Reduce EAGAIN retry cap 50→10 (10ms max serial queue block). Extracted
    maxWriteRetries constant. Root cause of BrainBar socket death.

H3: brain_search filter params (project, tag, importance_min) were advertised
    in tool schema but never applied. BrainDatabase.search() now builds
    dynamic WHERE clause. MCPRouter extracts and passes all 3 filters.

Also: 9 new tests (5 DB filter, 2 E2E router, stalled client, overlong path).
37 Swift + 921 Python = 958 tests passing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@EtanHey
Copy link
Copy Markdown
Owner Author

EtanHey commented Mar 18, 2026

@coderabbitai review

@EtanHey
Copy link
Copy Markdown
Owner Author

EtanHey commented Mar 18, 2026

@codex review

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 18, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 18, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR introduces search filtering capabilities and improves write retry handling. Changes include adding a configurable write retry limit constant, extending database chunk storage with tags support, implementing search filters for project/tag/importance parameters, updating the routing layer to support filtered searches, and adding comprehensive unit and integration tests for new functionality.

Changes

Cohort / File(s) Summary
Server Write Retry Logic
brain-bar/Sources/BrainBar/BrainBarServer.swift
Introduces maxWriteRetries static constant (value: 10) replacing hardcoded retry threshold; adds detailed logging when retry limit exceeded; initiates client disconnection upon threshold violation.
Database Layer Enhancements
brain-bar/Sources/BrainBar/BrainDatabase.swift
Adds optional tags parameter to insertChunk() with default value "[]"; extends search() with optional filters (project, tag, importanceMin); implements dynamic WHERE clause building and conditional parameter binding for filtered queries.
Router Layer Updates
brain-bar/Sources/BrainBar/MCPRouter.swift
Updates handleBrainSearch to parse importance_min from JSON as Double or Int; forwards new optional filter parameters (project, tag, importanceMin) to database search invocation.
Database Unit Tests
brain-bar/Tests/BrainBarTests/DatabaseTests.swift
Adds 5 new test cases: testSearchFiltersByProject, testSearchWithoutProjectReturnsAll, testSearchFiltersByImportanceMin, testSearchFiltersByTag, and testSearchCombinesFilters validating individual and combined filter behaviors.
Router Unit Tests
brain-bar/Tests/BrainBarTests/MCPRouterTests.swift
Adds 2 new test cases: testBrainSearchPassesProjectFilter and testBrainSearchPassesImportanceMinFilter; validates filter parameters propagate correctly through router to database and appear in results.
Socket Integration Tests
brain-bar/Tests/BrainBarTests/SocketIntegrationTests.swift
Adds 2 new integration tests: testServerDisconnectsStalledClient validates retry limit behavior with write-stalled clients; testRejectsOverlongSocketPath ensures server stability with invalid socket paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #87: Introduced the initial BrainBarServer EAGAIN write-retry and disconnectClient logic that this PR refactors with the new maxWriteRetries constant and enhanced logging.

Poem

🐰 Hops through tags and filters bright,
Retries capped at ten—what might!
Search parameters dance with glee,
Tests ensure what ought to be.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the two main changes: fixing the write retry cap (C1) and implementing search filter parameters (H3), matching the primary objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/brainbar-critical-bugs
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant