fix: search routing — stop file extensions hijacking queries#53
Conversation
…acking queries
Fixes 3 routing bugs that made BrainLayer unreliable as a knowledge source:
- B1/B2: _extract_file_path only triggers for ≤2 token queries (prevents
"how is auth in CLAUDE.md" from routing to file timeline)
- B3-B6: Remove broad recall signals ("what about", "worked on", "context for")
- B7: Increase effective_k for non-claude_code sources (youtube/whatsapp);
add project+source filters to FTS5; add idx_chunks_project index
Also adds format="compact" to brain_search (~40% token savings) and deletes
4 bad tests with zero assertions.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughPR removes legacy test script, introduces compact result formatting for brain_search with new format parameter, adds project-based filtering and indexing to vector store, enhances test coverage with new search routing tests and improved mocking, and applies cosmetic formatting improvements across multiple modules. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![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.
- ruff format across all src/ and tests/ (fixes pre-existing format drift) - Patch requests.get and VectorStore in test_mlx_health_recovery to prevent real Ollama connection attempts in CI (pre-existing flaky test) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/brainlayer/mcp/__init__.py (1)
1510-1583:⚠️ Potential issue | 🟠 Major
format="compact"is ignored on routed non-search paths.When routing to context/file/think/recall branches, the function returns verbose markdown regardless of
format, so compact callers still incur full token costs.🔧 Minimal guard to avoid silent contract mismatch
async def _brain_search( @@ ): @@ + compact_incompatible_route = ( + chunk_id is not None + or file_path is not None + or _extract_file_path(query) is not None + or _query_signals_current_context(query) + or _query_signals_think(query) + or _query_signals_recall(query) + ) + if format == "compact" and entity_id is None and compact_incompatible_route: + return _error_result( + "format='compact' is currently supported only for semantic/entity search routes." + ) + # Rule 1: chunk context expand
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/brainlayer/mcp/__init__.py`:
- Around line 272-277: The current construction for result in
src/brainlayer/mcp/__init__.py uses item.get("project", "unknown") and
item.get("source_file", "unknown"), which doesn't guard against keys present
with a None value; update the assignment for "project" and "source_file" in the
result dict (the block that builds result from item) to coalesce None to the
fallback (e.g., use the truthy coalescing pattern: use item.get("project") or
"unknown" and item.get("source_file") or "unknown") so compacted inputs that set
those keys to None will yield "unknown".
In `@tests/test_search_routing.py`:
- Around line 97-101: The test_docstring and assertion in
test_compact_result_has_required_fields disagree: the docstring lists required
fields "date", "importance", and "summary" but the assertion only checks
{"score","content","project","source_file"}; update the test to make them
consistent by either (A) expanding the required_keys set in
test_compact_result_has_required_fields to include "date", "importance", and
"summary" if _build_compact_item is expected to always provide them, or (B)
change the docstring to list only the four asserted fields if those three are
optional; reference the test function test_compact_result_has_required_fields
and the helper _build_compact_item when making the change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
scripts/test_extraction.pysrc/brainlayer/cli/__init__.pysrc/brainlayer/mcp/__init__.pysrc/brainlayer/pipeline/enrichment.pysrc/brainlayer/vector_store.pytests/test_kg_schema.pytests/test_mlx_health_recovery.pytests/test_ner_eval.pytests/test_recent_enrichment.pytests/test_search_routing.py
💤 Files with no reviewable changes (2)
- tests/test_ner_eval.py
- scripts/test_extraction.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). (3)
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
🧰 Additional context used
📓 Path-based instructions (4)
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use pytest for testing with ruff for linting and formatting: run
ruff check src/andruff format src/
Files:
tests/test_recent_enrichment.pytests/test_mlx_health_recovery.pytests/test_kg_schema.pytests/test_search_routing.py
src/brainlayer/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use Python package structure with Typer CLI for BrainLayer command-line interface in
src/brainlayer/
Files:
src/brainlayer/cli/__init__.pysrc/brainlayer/pipeline/enrichment.pysrc/brainlayer/mcp/__init__.pysrc/brainlayer/vector_store.py
src/brainlayer/mcp/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement MCP server with tools:
brain_search,brain_store,brain_recall(with legacybrainlayer_*aliases) with entrypointbrainlayer-mcp
Files:
src/brainlayer/mcp/__init__.py
src/brainlayer/vector_store.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/vector_store.py: Use sqlite-vec via APSW for storage invector_store.pywith WAL mode andbusy_timeout=5000
Store database at~/.local/share/brainlayer/brainlayer.dbwith sqlite-vec and WAL mode; use retry logic onSQLITE_BUSYerrors with each worker using its own connection
Files:
src/brainlayer/vector_store.py
🧠 Learnings (4)
📚 Learning: 2026-02-27T16:15:05.257Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T16:15:05.257Z
Learning: Applies to src/brainlayer/*enrich*.py : For enrichment, support both Ollama (`glm4`) and MLX backends via `BRAINLAYER_ENRICH_BACKEND` environment variable; set `"think": false` for GLM-4.7 speed optimization
Applied to files:
src/brainlayer/cli/__init__.pysrc/brainlayer/pipeline/enrichment.py
📚 Learning: 2026-02-27T16:15:05.257Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T16:15:05.257Z
Learning: Applies to src/brainlayer/*enrich*.py : Cache enrichment prompts at `~/.local/share/brainlayer/prompts/` and use `/tmp/brainlayer-enrichment.lock` for enrichment operation locking
Applied to files:
src/brainlayer/cli/__init__.pysrc/brainlayer/pipeline/enrichment.py
📚 Learning: 2026-02-27T16:15:05.257Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T16:15:05.257Z
Learning: Use environment variables for configurable backends (e.g., `BRAINLAYER_ENRICH_BACKEND`) to enable pluggable enrichment strategies
Applied to files:
src/brainlayer/cli/__init__.pysrc/brainlayer/pipeline/enrichment.py
📚 Learning: 2026-02-27T16:15:05.257Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T16:15:05.257Z
Learning: Applies to src/brainlayer/mcp/**/*.py : Implement MCP server with tools: `brain_search`, `brain_store`, `brain_recall` (with legacy `brainlayer_*` aliases) with entrypoint `brainlayer-mcp`
Applied to files:
src/brainlayer/mcp/__init__.py
🧬 Code graph analysis (2)
tests/test_mlx_health_recovery.py (1)
tests/test_kg_schema.py (1)
store(23-28)
tests/test_kg_schema.py (3)
tests/test_kg_standard.py (1)
store(25-30)tests/test_think_recall_integration.py (1)
store(19-23)src/brainlayer/vector_store.py (1)
_read_cursor(580-588)
🔇 Additional comments (18)
tests/test_recent_enrichment.py (1)
20-20: Assertion still covers both invocation styles.This check remains correct for both keyword and positional forwarding of
since_hours.tests/test_kg_schema.py (1)
78-92: Column expectation reflow is functionally unchanged.The strict equality checks remain intact and still protect against schema drift.
Also applies to: 99-113
src/brainlayer/cli/__init__.py (1)
875-876: CLI option reflow preserves behavior.No functional change in
enrichoption wiring from this edit.tests/test_mlx_health_recovery.py (1)
83-95: Good test isolation for enrichment recovery paths.Mocking
requests.getandVectorStorehere removes external dependencies and makes these tests deterministic.Also applies to: 124-135
src/brainlayer/pipeline/enrichment.py (1)
144-151: Formatting updates only; enrichment behavior stays unchanged.These edits are non-functional and keep runtime logic intact.
Also applies to: 839-840, 1115-1116, 1227-1228
src/brainlayer/mcp/__init__.py (2)
194-205: Good safeguard against filename-token routing hijack.The ≤2-token gate is a solid fix for semantic queries that merely mention filenames.
1882-1901: Compact_searchbranch is clean and efficient.Returning structured-only compact payloads here is a good implementation for token reduction.
src/brainlayer/vector_store.py (7)
114-114: LGTM!Formatting adjustment for the exponential backoff delay calculation. No functional change.
207-209: LGTM!Adding the
idx_chunks_projectindex supports efficient project-based filtering in queries. TheIF NOT EXISTSclause ensures safe migration for existing databases.
526-540: LGTM!Composite indexes on
(valid_from, valid_until)for bothkg_entitiesandkg_relationstables support efficient temporal validity filtering queries.
807-812: LGTM!The overfetch logic correctly identifies two scenarios where most KNN candidates will be discarded:
- Entity filtering (post-KNN)
- Rare source filtering (non-claude_code sources represent <0.01% of chunks)
The
n_results * 10multiplier with a cap at 1000 is a reasonable trade-off between recall and query cost.
1111-1116: LGTM!Adding
project_filterandsource_filterto the FTS5 query path ensures consistency with the vector search branch. This improves query efficiency by filtering at the SQL level rather than post-processing, and aligns with the existing handling of other filters liketag_filterandintent_filter.
2375-2390: LGTM!Reformatted parameter tuple improves readability with one parameter per line.
2449-2462: LGTM!Reformatted parameter tuple improves readability with one parameter per line.
tests/test_search_routing.py (4)
1-14: LGTM!Clear module docstring describing test coverage, and focused imports of the specific functions under test.
19-52: LGTM!Comprehensive test coverage for the file path extraction fix. Tests correctly verify:
- Multi-word queries with embedded filenames should NOT trigger file routing
- Single filenames and short (≤2 token) file queries should still extract paths
The assertion messages are helpful for debugging failures.
57-89: LGTM!Excellent test coverage for recall signal filtering:
- Negative tests ensure broad terms don't trigger false positives
- Positive tests verify the remaining signals still work
- Direct validation of
_RECALL_SIGNALScontents prevents regression
103-163: LGTM!The remaining compact format tests are well-designed:
test_compact_result_drops_verbose_fields: Validates all expected fields are excludedtest_compact_content_truncated_to_500: Tests the 500-char truncation behaviortest_compact_fewer_tokens_than_full: Validates overall key count reductionThe
_build_compact_itemhelper correctly imports the production function, ensuring test behavior stays synchronized with the implementation.
…rtion alignment - _build_compact_result: use `or "unknown"` instead of `.get(key, "unknown")` to handle explicit None values for project/source_file - Test docstring now matches assertion (required vs optional fields) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DROP VIEW + CREATE VIEW is not atomic — concurrent VectorStore init threads can race between the DROP and CREATE, causing "already exists" errors. Wrap in try/except since the desired state (view exists) is achieved either way. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
_extract_file_pathnow only triggers for queries with ≤2 tokens. Queries like "how is auth in CLAUDE.md" or "package.json best practices" no longer get hijacked to file timeline — they route to semantic search as intended._RECALL_SIGNALS. Only "history of", "discussed about", "thought about" remain.effective_know bumps ton_results * 10for non-claude_code sources. Added project+source filters to FTS5 query. Added missingidx_chunks_projectindex.brain_searchreturning only score, content (500 chars), project, source_file, date, importance, summary. ~40% token savings.scripts/test_extraction.py(3 no-assertion tests) andtest_seed_baseline_reports_metrics(always-pass observability test).Test plan
tests/test_search_routing.pycovering all bug fixes🤖 Generated with Claude Code
Summary by CodeRabbit