feat: tune BrainLayer read path PRAGMAs and default MMR off#305
Conversation
![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 |
|
@greptile-apps review |
|
@codex review |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
📝 WalkthroughWalkthroughThis PR makes MMR reranking behavior and SQLite connection tuning configurable via environment variables. MMR is disabled by default, and database pragmas are set to conservative values. Both changes include comprehensive tests validating environment-variable parsing, fallback behavior on invalid input, and observable configuration effects. ChangesEnvironment-driven configuration for MMR and SQLite PRAGMA tuning
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
✅ Actions performedReview triggered.
|
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/brainlayer/search_repo.py`:
- Around line 35-38: The current parsing of BRAINLAYER_MMR_LAMBDA assigns any
float (including nan, inf, negatives) to _MMR_LAMBDA; change the try/except
block that sets _MMR_LAMBDA so after converting the env var to float you
validate with math.isfinite(value) and then clamp the value into the [0.0, 1.0]
range (e.g., value = max(0.0, min(1.0, value))); if parsing fails or the value
is not finite, fall back to 1.0; keep the symbol name _MMR_LAMBDA and the env
key "BRAINLAYER_MMR_LAMBDA" so the change is localized to that assignment.
🪄 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: cf52f1fc-c2d2-4eae-a0d4-46f48758e908
📒 Files selected for processing (5)
src/brainlayer/search_repo.pysrc/brainlayer/vector_store.pytests/test_hybrid_search.pytests/test_mmr_default_off.pytests/test_vector_store_pragma_tuning.py
📜 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). (4)
- GitHub Check: Cursor Bugbot
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Flag risky DB or concurrency changes explicitly and do not hand-wave lock behavior
Enforce one-write-at-a-time concurrency constraint; reads are safe but brain_digest is write-heavy and must not run in parallel with other MCP work
Run pytest before claiming behavior changed safely; current test suite has 929 tests
**/*.py: Usepaths.py:get_db_path()for all database path resolution; all scripts and CLI must use this function rather than hardcoding paths
When performing bulk database operations: stop enrichment workers first, checkpoint WAL before and after, drop FTS triggers before bulk deletes, batch deletes in 5-10K chunks, and checkpoint every 3 batches
Files:
tests/test_vector_store_pragma_tuning.pytests/test_hybrid_search.pytests/test_mmr_default_off.pysrc/brainlayer/search_repo.pysrc/brainlayer/vector_store.py
src/brainlayer/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/**/*.py: Use retry logic onSQLITE_BUSYerrors; each worker must use its own database connection to handle concurrency safely
Classification must preserveai_code,stack_trace, anduser_messageverbatim; skipnoiseentries entirely and summarizebuild_loganddir_listingentries (structure only)
Use AST-aware chunking via tree-sitter; never split stack traces; mask large tool output
For enrichment backend selection: use Groq as primary backend (cloud, configured in launchd plist), Gemini as fallback viaenrichment_controller.py, and Ollama as offline last-resort; allow override viaBRAINLAYER_ENRICH_BACKENDenv var
Configure enrichment rate viaBRAINLAYER_ENRICH_RATEenvironment variable (default 0.2 = 12 RPM)
Implement chunk lifecycle columns:superseded_by,aggregated_into,archived_aton chunks table; exclude lifecycle-managed chunks from default search; allowinclude_archived=Trueto show history
Implementbrain_supersedewith safety gate for personal data (journals, notes, health/finance); use soft-delete forbrain_archivewith timestamp
Addsupersedesparameter tobrain_storefor atomic store-and-replace operations
Run linting and formatting with:ruff check src/ && ruff format src/
Run tests withpytest
UsePRAGMA wal_checkpoint(FULL)before and after bulk database operations to prevent WAL bloat
Files:
src/brainlayer/search_repo.pysrc/brainlayer/vector_store.py
🪛 OpenGrep (1.21.0)
tests/test_vector_store_pragma_tuning.py
[ERROR] 7-7: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
src/brainlayer/vector_store.py
[ERROR] 202-202: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 1201-1201: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
[ERROR] 1202-1202: SQL query built via f-string passed to execute()/executemany(). Use parameterized queries with placeholders instead.
(coderabbit.sql-injection.python-fstring-execute)
🔇 Additional comments (6)
src/brainlayer/vector_store.py (2)
202-202: OpenGrep f-string SQL warnings here are false positives.
PRAGMAstatements in SQLite do not support bound parameters for their values, so an f-string is required. The interpolated values come from_wal_autocheckpoint_pages(),_read_mmap_bytes(), and_read_cache_size_kb(), all of which return ints derived viaint(...)with safe fallbacks — no untrusted string flows into the SQL. No change needed; flagging only to document the rationale for ignoring the static-analysis hits.Also applies to: 1200-1202
74-91: LGTM!tests/test_vector_store_pragma_tuning.py (1)
11-84: LGTM!src/brainlayer/search_repo.py (1)
484-485: LGTM!tests/test_hybrid_search.py (1)
434-436: LGTM!Also applies to: 492-494
tests/test_mmr_default_off.py (1)
1-55: LGTM!
![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: 348cee945a
ℹ️ 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".
| try: | ||
| _MMR_LAMBDA = float(os.environ.get("BRAINLAYER_MMR_LAMBDA", "1.0")) | ||
| except (TypeError, ValueError): | ||
| _MMR_LAMBDA = 1.0 |
There was a problem hiding this comment.
BRAINLAYER_MMR_LAMBDA is parsed with float(...), so non-finite values like nan/inf are accepted and do not hit the fallback. With nan, the reranker enters the MMR path (nan >= 1.0 is false) and computes nan MMR scores, so candidate selection no longer reflects relevance/diversity and retrieval ordering can silently degrade for all queries in that process. Validate the parsed value (finite and bounded) and fall back to the default when invalid.
Useful? React with 👍 / 👎.
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50389f500c
ℹ️ 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".
|
|
||
|
|
||
| def _wal_autocheckpoint_pages() -> int: | ||
| return max(_int_env("BRAINLAYER_WAL_AUTOCHECKPOINT", 10_000), 0) |
There was a problem hiding this comment.
_wal_autocheckpoint_pages() currently clamps negative BRAINLAYER_WAL_AUTOCHECKPOINT values to 0, and SQLite treats wal_autocheckpoint <= 0 as disabling auto-checkpointing. That means a common misconfiguration like -1 silently turns checkpointing off instead of falling back to the safe default, which can rapidly grow the WAL file under write load (a known stability risk in this repo). Treat non-positive values as invalid and fall back to 10000 rather than passing 0 through.
Useful? React with 👍 / 👎.
1 participant
Summary
mmap_size=30000000000andcache_size=-64000, withBRAINLAYER_READ_MMAP_BYTESandBRAINLAYER_READ_CACHE_KBoverrides.wal_autocheckpoint=10000, withBRAINLAYER_WAL_AUTOCHECKPOINToverride.BRAINLAYER_MMR_LAMBDA=1.0; lower values still opt into the existing MMR path. Triple RRF is preserved unchanged.R3 Stress-Test Evidence
Headline from the read-path redesign:
brain_searchmedian16930ms -> 33ms(~510x), p9511337ms -> 2384ms(~4.6x). Triple RRF remains unchanged.Phase 4 Future Work
The remaining 8s tail outliers and sqlite-vec brute-force KNN floor are Phase 4 future work. The likely durable fix is HNSW/vector-backend replacement after TechGym, not broadening this PR.
Repro Commands
Verification
.venv/bin/python -m pytest tests/test_vector_store_pragma_tuning.py tests/test_mmr_default_off.py -vproduced expected failures before implementation (4 failed, 1 passed, 1 skipped)..venv/bin/python -m pytest tests/test_vector_store_pragma_tuning.py tests/test_mmr_default_off.py -v->8 passed..venv/bin/python -m pytest tests/test_hybrid_search.py tests/test_vector_store_pragma_tuning.py tests/test_mmr_default_off.py -v->24 passed..venv/bin/ruff check src testspassed;.venv/bin/ruff format --check src testspassed.2054 passed, 9 skipped, 75 deselected, 1 xfailed), MCP registration (3 passed), isolated eval/hook routing (32 passed), bun stale-index suite (1 pass), FTS5 determinism shell regression passed./tmp/readpath-r3-10gb.dbcopy:BRAINLAYER_DB=/tmp/readpath-r3-10gb.db .venv/bin/python -m pytest tests/test_eval_baselines.py -m live -v->32 passed, 2 failed, 5 xfailed, 2 xpassed; the two failures match the accepted pre-existing R4 prompt allowances: no recent-week chunks in the fixture copy and generic Python entity injection.~/.local/share/brainlayer/brainlayer.dbwas blocked by active DB locks from enrichment/drain (apsw.BusyError).tests/test_eval_framework.pypassed (11 passed).Local Review
cr review --plainafter fixes reports only.gemini/settings.json, which is untracked and not included in this branch.Note
Medium Risk
Medium risk because it changes search ranking behavior (MMR reranking now disabled by default) and adjusts SQLite connection PRAGMAs, which can affect performance and operational characteristics across deployments.
Overview
Tunes BrainLayer’s SQLite read/write connection settings and makes MMR reranking opt-in.
Read-only connections now set
PRAGMA mmap_sizeandPRAGMA cache_size, and writer init setsPRAGMA wal_autocheckpoint; all three are configurable viaBRAINLAYER_READ_MMAP_BYTES,BRAINLAYER_READ_CACHE_KB, andBRAINLAYER_WAL_AUTOCHECKPOINTwith safe parsing/fallbacks.Hybrid search MMR reranking is now off by default (
_MMR_LAMBDA=1.0), configurable viaBRAINLAYER_MMR_LAMBDA, and short-circuits to skip embedding loads when disabled; tests were added/updated to cover env overrides, clamping/invalid values, and PRAGMA application.Reviewed by Cursor Bugbot for commit 50389f5. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Tune BrainLayer read path SQLite PRAGMAs and disable MMR reranking by default
VectorStore._get_read_connnow setPRAGMA mmap_size(default 30 GB) andPRAGMA cache_size(default -64000 KB); writer connections setPRAGMA wal_autocheckpoint(default 10000 pages). All values are overridable via environment variables._MMR_LAMBDAinsearch_repo.pynow defaults to1.0(MMR off), readable fromBRAINLAYER_MMR_LAMBDA; invalid or non-finite values fall back to1.0.SearchMixin._mmr_rerank_scored_resultsshort-circuits and skips embedding loads when_MMR_LAMBDA >= 1.0.BRAINLAYER_MMR_LAMBDAto a value below1.0.Macroscope summarized 50389f5.
Summary by CodeRabbit
New Features
BRAINLAYER_MMR_LAMBDAenvironment variable.BRAINLAYER_WAL_AUTOCHECKPOINT,BRAINLAYER_READ_MMAP_BYTES,BRAINLAYER_READ_CACHE_KB).Tests