Skip to content

feat: search quality improvements + Groq rate limiter#68

Merged
EtanHey merged 5 commits intomainfrom
feature/search-quality-and-enrichment-fix
Mar 3, 2026
Merged

feat: search quality improvements + Groq rate limiter#68
EtanHey merged 5 commits intomainfrom
feature/search-quality-and-enrichment-fix

Conversation

@EtanHey
Copy link
Copy Markdown
Owner

@EtanHey EtanHey commented Mar 3, 2026

Summary

  • FTS5 expansion (A2): chunks_fts now indexes summary, tags, resolved_query alongside content — keyword searches match enrichment metadata (biggest relevance win, coachClaude 0.4666→0.7+)
  • Entity-aware routing (A4): Auto-detect KG entity names in queries → route to kg_hybrid_search for combined chunk + fact retrieval (was dead code, now connected)
  • Post-RRF reranking (A5): Importance boost (up to 1.5x) + recency boost (30-day half-life decay) after RRF scoring
  • format=format bug fix (A6): Default search path was passing Python builtin format() instead of detail param
  • Groq rate limiter (A1): 2s minimum delay between Groq API calls to avoid free tier 429 spam (was 4 chunks/hr, target 25-30/hr)
  • Enrichment source priority: Claude Code chunks enriched before YouTube backlog

Test plan

  • 20 new tests covering all changes (test_search_quality.py + test_groq_rate_limit.py)
  • 695 total tests passing, 0 regressions
  • Verify Groq enrichment runs without 429 spam after deploy
  • Verify FTS5 rebuild on existing production DB (auto-detects old schema)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Entity-aware search routing using knowledge-graph matches
    • Extended full-text search to include summaries, tags, and resolved queries
  • Improvements

    • Dynamic ranking adjusted by item importance and recency
    • Prioritized ordering of manual/code content sources
    • API rate limiting for more stable external calls
  • Bug Fixes

    • Fixed default search parameter handling to preserve expected output
  • Tests

    • Added end-to-end and rate-limit tests covering search and enrichment behaviors

Search quality (tasks A2, A4, A5, A6):
- FTS5 expansion: index summary, tags, resolved_query alongside content
  for better keyword matches on enrichment metadata
- Entity-aware routing: auto-detect KG entity names in queries and route
  to kg_hybrid_search for combined chunk + fact retrieval
- Post-RRF reranking: boost results by importance (up to 1.5x) and
  recency (30-day half-life exponential decay)
- Fix format=format bug: default search path was passing Python builtin
  format() instead of detail parameter

Enrichment fix (task A1):
- Add 2s rate limit between Groq API calls to avoid 429 spam on free tier
- Prioritize Claude Code chunks over YouTube in enrichment queue
- Source priority ordering in get_unenriched_chunks

20 new tests, 695 total passing, 0 regressions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 3, 2026

Warning

Rate limit exceeded

@EtanHey has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 26 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 1655c89 and d099cfb.

📒 Files selected for processing (4)
  • src/brainlayer/mcp/search_handler.py
  • src/brainlayer/pipeline/enrichment.py
  • tests/test_groq_rate_limit.py
  • tests/test_search_quality.py
📝 Walkthrough

Walkthrough

Adds entity-aware routing to search (detects KG entities and routes to kg_hybrid_search), expands FTS5 to index enriched fields, applies post-RRF boosting by importance and recency, prioritizes chunk sources, and enforces Groq API rate limiting; accompanying tests validate these behaviors.

Changes

Cohort / File(s) Summary
Entity-aware search routing
src/brainlayer/mcp/search_handler.py
Adds _detect_entities (bigram + capitalized-word extraction) and integrates entity detection into _brain_search, routing matched queries to kg_hybrid_search, expanding _brain_search/_search signatures to accept routing/filter params and detail, preserving backward-compat format behavior.
FTS5 schema expansion & migration
src/brainlayer/vector_store.py
Revises chunks_fts to multi-column FTS5 (content, summary, tags, resolved_query, chunk_id), adds detection/rebuild of old single-column schema, updates triggers and backfill to sync enriched fields on insert/update/delete.
Search result reranking
src/brainlayer/search_repo.py
Applies post-processing boost to hybrid_search results combining importance-based multiplier (mapped 0–10 → 1.0–1.5x) and recency exponential decay (30-day half-life) before final sort.
Chunk source prioritization
src/brainlayer/session_repo.py
Extends get_unenriched_chunks to select source and orders results via a CASE-based source priority (claude_code/manual → digest/others → whatsapp → youtube) ahead of rowid DESC; returns source in chunk dicts.
Groq API rate limiting
src/brainlayer/pipeline/enrichment.py
Adds GROQ_RATE_LIMIT_DELAY (env-driven, default 2.0s) and _groq_last_call timestamp; call_groq enforces minimum delay using monotonic time before issuing requests.
Tests
tests/test_groq_rate_limit.py, tests/test_search_quality.py
New tests covering Groq rate limiting and overrides, FTS5 enrichment/trigger behavior, entity detection & routing, post-RRF reranking (importance/recency), and chunk source prioritization; includes DB fixtures and deterministic embeddings.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant SearchHandler as search_handler\n(_brain_search/_search)
    participant Detector as _detect_entities
    participant KGDB as KG Tables\n(kg_entities, kg_entities_fts)
    participant Vector as vector_store\n(kg_hybrid_search)
    
    Client->>SearchHandler: submit query
    SearchHandler->>Detector: extract candidate entities (bigrams, caps)
    Detector->>KGDB: lookup exact / FTS matches
    alt entity found
        KGDB-->>Detector: entity record(s)
        Detector-->>SearchHandler: matched entity list
        SearchHandler->>Vector: kg_hybrid_search(query, entity_name, routing params)
        Vector-->>SearchHandler: chunks + KG facts (formatted)
        SearchHandler-->>Client: entity-aware results
    else no entity
        KGDB-->>Detector: no match
        Detector-->>SearchHandler: []
        SearchHandler->>SearchHandler: fallback to standard hybrid_search/_search path
        SearchHandler-->>Client: standard search results
    end
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰🌿
I sniff the query, twitch my nose,
Bigrams hop where knowledge grows,
FTS and facts come into view,
Scores age wise, Groq ticks true—
Hoppy search, here’s help for you!

🚥 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 PR title accurately summarizes the two primary objectives: search quality improvements (FTS5 expansion, entity-aware routing, post-RRF reranking, format bug fix) and Groq rate limiter implementation.
Docstring Coverage ✅ Passed Docstring coverage is 93.55% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/search-quality-and-enrichment-fix

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.

EtanHey and others added 2 commits March 3, 2026 12:43
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 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/search_handler.py`:
- Around line 225-231: _brain_search currently skips _search and calls
kg_hybrid_search without forwarding active filters, causing different semantics;
modify the kg_hybrid_search call in _brain_search to pass through the same
filter parameters that _search uses (e.g., content_type, source, tag, intent,
importance_min, date_from, date_to, sentiment and any other active filter keys)
by extracting them from the incoming filters/context and supplying them as named
arguments to kg_hybrid_search (keep existing args like query_embedding,
query_text, n_results, entity_name, project_filter) so entity-aware routes honor
the same filters as non-entity searches.

In `@src/brainlayer/pipeline/enrichment.py`:
- Around line 95-97: The shared _groq_last_call and GROQ_RATE_LIMIT_DELAY
rate-limiting logic is not thread-safe; protect reads/updates to _groq_last_call
with a lock and perform the wait inside the critical section to enforce spacing
across threads. Add a module-level threading.Lock (e.g., _groq_lock) and wrap
the check/update/sleep sequence that uses _groq_last_call and
GROQ_RATE_LIMIT_DELAY (the same logic around the GROQ call present near the top
with _groq_last_call and the duplicate block around lines ~535-547) so only one
thread can compute elapsed, sleep the required remaining time, update
_groq_last_call using time.monotonic(), then release the lock before making the
API request.

In `@tests/test_groq_rate_limit.py`:
- Around line 56-69: The test test_rate_limit_delay_env_override currently only
checks os.environ and never validates the module-level constant; update it to
reload the enrichment module and assert the module constant matches the env
value: after setting os.environ["BRAINLAYER_GROQ_RATE_DELAY"] = "5.0", call
importlib.reload(enrichment) and assert enrichment.GROQ_RATE_LIMIT_DELAY (or the
actual module-level constant name in brainlayer.pipeline.enrichment) equals 5.0,
keeping the existing try/finally that restores the original env.

In `@tests/test_search_quality.py`:
- Around line 329-360: The test test_entity_routing_calls_kg_hybrid_search is
calling store.kg_hybrid_search directly so it never exercises the routing logic
in _brain_search; change the test to call the public brain search entrypoint
(e.g., store._brain_search or the exposed brain_search wrapper) with the same
setup and mock the kg_hybrid_search method (or spy on store.kg_hybrid_search) to
assert it was invoked when the query contains the entity; keep the existing DB
fixtures (kg_entities, kg_entity_chunks, chunk) and ensure the test passes
query_text="Michal Cohen meetings" and query_embedding=mock_embed(...) into the
brain search call, then assert that store.kg_hybrid_search was called (and/or
that the brain search returns the entity-linked chunk) instead of calling
kg_hybrid_search directly.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f79dc04 and 819fd95.

📒 Files selected for processing (7)
  • src/brainlayer/mcp/search_handler.py
  • src/brainlayer/pipeline/enrichment.py
  • src/brainlayer/search_repo.py
  • src/brainlayer/session_repo.py
  • src/brainlayer/vector_store.py
  • tests/test_groq_rate_limit.py
  • tests/test_search_quality.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.11)
  • GitHub Check: test (3.12)
  • GitHub Check: test (3.13)
🧰 Additional context used
📓 Path-based instructions (6)
**/*test*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Run tests with pytest

Files:

  • tests/test_groq_rate_limit.py
  • tests/test_search_quality.py
src/brainlayer/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use Python package structure with Typer CLI located in src/brainlayer/

Files:

  • src/brainlayer/pipeline/enrichment.py
  • src/brainlayer/vector_store.py
  • src/brainlayer/search_repo.py
  • src/brainlayer/session_repo.py
  • src/brainlayer/mcp/search_handler.py
**/enrichment*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/enrichment*.py: Use MLX (Qwen2.5-Coder-14B-Instruct-4bit) as primary enrichment backend on Apple Silicon (port 8080) with Ollama (glm-4.7-flash) fallback on port 11434
Auto-switch enrichment backend from MLX to Ollama after 3 consecutive MLX failures; allow override with BRAINLAYER_ENRICH_BACKEND environment variable

Files:

  • src/brainlayer/pipeline/enrichment.py
src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Lint and format Python code using ruff check src/ and ruff format src/

Files:

  • src/brainlayer/pipeline/enrichment.py
  • src/brainlayer/vector_store.py
  • src/brainlayer/search_repo.py
  • src/brainlayer/session_repo.py
  • src/brainlayer/mcp/search_handler.py
**/vector_store.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use APSW with sqlite-vec for vector storage (in vector_store.py)

Files:

  • src/brainlayer/vector_store.py
**/mcp/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Implement MCP tools: brain_search, brain_store, brain_recall, brain_entity, brain_expand, brain_update, brain_digest, brain_get_person with legacy brainlayer_* aliases

Files:

  • src/brainlayer/mcp/search_handler.py
🧠 Learnings (4)
📚 Learning: 2026-03-02T21:14:04.731Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T21:14:04.731Z
Learning: Applies to **/enrichment*.py : Auto-switch enrichment backend from MLX to Ollama after 3 consecutive MLX failures; allow override with `BRAINLAYER_ENRICH_BACKEND` environment variable

Applied to files:

  • src/brainlayer/pipeline/enrichment.py
📚 Learning: 2026-03-02T21:14:04.731Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T21:14:04.731Z
Learning: Applies to scripts/**/*.py : Drop FTS triggers (especially `chunks_fts_delete`) before bulk deletes and recreate after — FTS triggers cause massive performance degradation on large datasets

Applied to files:

  • src/brainlayer/vector_store.py
📚 Learning: 2026-03-02T21:14:04.731Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T21:14:04.731Z
Learning: Applies to scripts/**/*.py : Never delete from `chunks` table while FTS trigger is active on large datasets

Applied to files:

  • src/brainlayer/vector_store.py
📚 Learning: 2026-03-02T21:14:04.731Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T21:14:04.731Z
Learning: Applies to **/mcp/**/*.py : Implement MCP tools: `brain_search`, `brain_store`, `brain_recall`, `brain_entity`, `brain_expand`, `brain_update`, `brain_digest`, `brain_get_person` with legacy `brainlayer_*` aliases

Applied to files:

  • src/brainlayer/mcp/search_handler.py
🧬 Code graph analysis (4)
tests/test_groq_rate_limit.py (2)
src/brainlayer/pipeline/enrichment.py (1)
  • call_groq (525-581)
src/brainlayer/session_repo.py (1)
  • get_unenriched_chunks (21-86)
tests/test_search_quality.py (4)
src/brainlayer/vector_store.py (1)
  • close (689-698)
src/brainlayer/mcp/search_handler.py (3)
  • _embed (717-718)
  • _embed (752-753)
  • _detect_entities (29-97)
src/brainlayer/search_repo.py (1)
  • hybrid_search (335-580)
src/brainlayer/kg_repo.py (1)
  • kg_hybrid_search (769-818)
src/brainlayer/search_repo.py (1)
src/brainlayer/dashboard/search.py (1)
  • score (53-78)
src/brainlayer/mcp/search_handler.py (5)
src/brainlayer/vector_store.py (1)
  • _read_cursor (546-548)
src/brainlayer/_helpers.py (1)
  • _escape_fts5_query (35-54)
src/brainlayer/mcp/_shared.py (4)
  • _get_vector_store (16-24)
  • _normalize_project_name (92-142)
  • _get_embedding_model (27-34)
  • _build_compact_result (239-250)
src/brainlayer/cli/__init__.py (1)
  • _normalize_project_name (140-162)
src/brainlayer/kg_repo.py (1)
  • kg_hybrid_search (769-818)
🔇 Additional comments (4)
src/brainlayer/session_repo.py (1)

54-67: Source-priority ordering and source propagation look correct.

The prioritization logic and returned source field are consistent with the intended enrichment queue behavior.

Also applies to: 83-83

src/brainlayer/search_repo.py (1)

542-568: Post-RRF reranking block is robust and well-guarded.

The boost application is bounded and gracefully skips malformed temporal metadata without breaking ranking.

src/brainlayer/vector_store.py (1)

186-234: FTS5 schema expansion and migration/backfill path are consistent.

The rebuild detection, trigger synchronization, and full-column backfill align with the expanded FTS search surface.

Also applies to: 519-526

src/brainlayer/mcp/search_handler.py (1)

301-301: Good fix on the default fallback argument forwarding.

Switching to detail=detail closes the format=format bug in the default path.

Comment on lines +225 to +231
kg_results = store.kg_hybrid_search(
query_embedding=query_embedding,
query_text=query,
n_results=num_results,
entity_name=entity_name,
project_filter=normalized_project,
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Entity-aware route drops active search filters.

When entity detection triggers, _brain_search bypasses _search and calls kg_hybrid_search without forwarding filters (content_type, source, tag, intent, importance_min, date_from/date_to, sentiment). This changes query semantics unexpectedly.

🔧 Suggested fix
-            kg_results = store.kg_hybrid_search(
+            if source == "all":
+                source_filter = None
+            elif source:
+                source_filter = source
+            else:
+                source_filter = None
+
+            kg_results = store.kg_hybrid_search(
                 query_embedding=query_embedding,
                 query_text=query,
                 n_results=num_results,
                 entity_name=entity_name,
                 project_filter=normalized_project,
+                content_type_filter=content_type,
+                source_filter=source_filter,
+                tag_filter=tag,
+                intent_filter=intent,
+                importance_min=importance_min,
+                date_from=date_from,
+                date_to=date_to,
+                sentiment_filter=sentiment,
             )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/brainlayer/mcp/search_handler.py` around lines 225 - 231, _brain_search
currently skips _search and calls kg_hybrid_search without forwarding active
filters, causing different semantics; modify the kg_hybrid_search call in
_brain_search to pass through the same filter parameters that _search uses
(e.g., content_type, source, tag, intent, importance_min, date_from, date_to,
sentiment and any other active filter keys) by extracting them from the incoming
filters/context and supplying them as named arguments to kg_hybrid_search (keep
existing args like query_embedding, query_text, n_results, entity_name,
project_filter) so entity-aware routes honor the same filters as non-entity
searches.

Comment thread src/brainlayer/pipeline/enrichment.py
Comment thread tests/test_groq_rate_limit.py
Comment thread tests/test_search_quality.py Outdated
Comment on lines +329 to +360
def test_entity_routing_calls_kg_hybrid_search(self, store, mock_embed):
"""When entities detected, brain_search should use kg_hybrid_search."""
# Insert entity
cursor = store.conn.cursor()
cursor.execute(
"""INSERT INTO kg_entities (id, entity_type, name, metadata, created_at)
VALUES (?, ?, ?, '{}', datetime('now'))""",
("ent-route-1", "person", "Michal Cohen"),
)
cursor.execute(
"INSERT INTO kg_entities_fts (name, metadata, entity_id) VALUES (?, '{}', ?)",
("Michal Cohen", "ent-route-1"),
)
# Insert a chunk linked to this entity
_insert_chunk(
store, "ent-chunk-1",
content="Michal Cohen prefers morning meetings before 10am",
importance=7.0,
created_at="2026-03-01T00:00:00",
)
cursor.execute(
"INSERT INTO kg_entity_chunks (entity_id, chunk_id, relevance) VALUES (?, ?, ?)",
("ent-route-1", "ent-chunk-1", 0.95),
)

# The search should find the entity-linked chunk
results = store.kg_hybrid_search(
query_embedding=mock_embed("Michal Cohen meetings"),
query_text="Michal Cohen meetings",
n_results=5,
entity_name="Michal Cohen",
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

test_entity_routing_calls_kg_hybrid_search does not actually test routing.

It calls store.kg_hybrid_search(...) directly, so _brain_search entity-detection/routing logic is never exercised. The test name and coverage intent are currently misleading.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_search_quality.py` around lines 329 - 360, The test
test_entity_routing_calls_kg_hybrid_search is calling store.kg_hybrid_search
directly so it never exercises the routing logic in _brain_search; change the
test to call the public brain search entrypoint (e.g., store._brain_search or
the exposed brain_search wrapper) with the same setup and mock the
kg_hybrid_search method (or spy on store.kg_hybrid_search) to assert it was
invoked when the query contains the entity; keep the existing DB fixtures
(kg_entities, kg_entity_chunks, chunk) and ensure the test passes
query_text="Michal Cohen meetings" and query_embedding=mock_embed(...) into the
brain search call, then assert that store.kg_hybrid_search was called (and/or
that the brain search returns the entity-linked chunk) instead of calling
kg_hybrid_search directly.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (3)
tests/test_groq_rate_limit.py (1)

53-69: ⚠️ Potential issue | 🟠 Major

Validate the loaded delay constant, not just the env var string.

Line 63 only checks os.environ, so this test still passes if brainlayer.pipeline.enrichment.GROQ_RATE_LIMIT_DELAY was already loaded with a stale value. Reload the module and assert the constant directly.

🔧 Suggested fix
     def test_rate_limit_delay_env_override(self):
         """BRAINLAYER_GROQ_RATE_DELAY env var should override the default delay."""
         import os
+        from importlib import reload
+        from brainlayer.pipeline import enrichment

         original = os.environ.get("BRAINLAYER_GROQ_RATE_DELAY")
         try:
             os.environ["BRAINLAYER_GROQ_RATE_DELAY"] = "5.0"
-            # Need to reload to pick up env var
-            # Check the module-level constant respects the env var
-            # (This tests the env var mechanism, not the reloaded value)
-            assert float(os.environ["BRAINLAYER_GROQ_RATE_DELAY"]) == 5.0
+            enrichment = reload(enrichment)
+            assert enrichment.GROQ_RATE_LIMIT_DELAY == 5.0
         finally:
             if original is None:
                 os.environ.pop("BRAINLAYER_GROQ_RATE_DELAY", None)
             else:
                 os.environ["BRAINLAYER_GROQ_RATE_DELAY"] = original
+            reload(enrichment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_groq_rate_limit.py` around lines 53 - 69, The test currently only
asserts the environment variable string instead of the module constant; modify
test_rate_limit_delay_env_override to set
os.environ["BRAINLAYER_GROQ_RATE_DELAY"]="5.0", reload the enrichment module
(e.g. import importlib; importlib.reload(brainlayer.pipeline.enrichment) or
import the module after setting the env), then assert that
brainlayer.pipeline.enrichment.GROQ_RATE_LIMIT_DELAY == 5.0 (or float(...)
equals 5.0); ensure you restore the original env var in the finally block as
before.
src/brainlayer/mcp/search_handler.py (1)

230-236: ⚠️ Potential issue | 🟠 Major

Entity-aware branch drops active filters before kg_hybrid_search.

Line 230 routes through kg_hybrid_search, but it does not forward current filters (content_type, source, tag, intent, importance_min, date_from, date_to, sentiment). This changes query semantics versus the default _search path.

🔧 Suggested fix
-            kg_results = store.kg_hybrid_search(
+            if source == "all":
+                source_filter = None
+            elif source:
+                source_filter = source
+            else:
+                source_filter = None
+
+            kg_results = store.kg_hybrid_search(
                 query_embedding=query_embedding,
                 query_text=query,
                 n_results=num_results,
                 entity_name=entity_name,
                 project_filter=normalized_project,
+                content_type_filter=content_type,
+                source_filter=source_filter,
+                tag_filter=tag,
+                intent_filter=intent,
+                importance_min=importance_min,
+                date_from=date_from,
+                date_to=date_to,
+                sentiment_filter=sentiment,
             )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/brainlayer/mcp/search_handler.py` around lines 230 - 236, The
entity-aware branch calls store.kg_hybrid_search without forwarding the active
filter parameters, which changes semantics compared to the default _search path;
update the store.kg_hybrid_search call in search_handler.py (the block invoking
store.kg_hybrid_search with query_embedding, query_text/query,
n_results/num_results, entity_name, project_filter/normalized_project) to pass
the current filter arguments (content_type, source, tag, intent, importance_min,
date_from, date_to, sentiment) so that kg_hybrid_search receives the same
filters as _search and preserves query semantics.
tests/test_search_quality.py (1)

360-394: ⚠️ Potential issue | 🟠 Major

This test does not verify routing through _brain_search.

Line 387 calls store.kg_hybrid_search(...) directly, so entity-detection/routing logic in brainlayer.mcp.search_handler._brain_search is never exercised. Rename or, preferably, route through _brain_search and assert kg_hybrid_search was invoked.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_search_quality.py` around lines 360 - 394, The test currently
calls kg_hybrid_search directly so routing logic in _brain_search is not
exercised; modify the test to call brainlayer.mcp.search_handler._brain_search
(or the public entry that triggers it) with the same inputs (query_text and
query_embedding) and use a spy/patch on the Store.kg_hybrid_search method
(reference symbol: kg_hybrid_search) to assert it was invoked and returned the
expected results; ensure the existing DB setup (kg_entities, kg_entities_fts,
kg_entity_chunks, and _insert_chunk) remains the same and replace the direct
call to store.kg_hybrid_search with a call to _brain_search, then assert the spy
was called and that the returned payload includes "chunks" and "facts".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/test_groq_rate_limit.py`:
- Around line 20-45: The test relies on global mutable state
enrichment._groq_last_call so reset or patch that state at the start of
test_call_groq_sleeps_between_calls to isolate timing (e.g., set
enrichment._groq_last_call = 0 or patch it via monkeypatch) before invoking
enrichment.call_groq three times; update the test to assign or patch
enrichment._groq_last_call inside the with-block (or at test start) so previous
tests can't affect the timing assertions.
- Around line 102-108: The test currently only checks ordering when both
"claude_code" and "youtube" appear, allowing a vacuous pass if either is
missing; update the test in tests/test_groq_rate_limit.py around the use of
store.get_unenriched_chunks and the sources list to first assert both sources
are present (e.g., assert "claude_code" in sources and assert "youtube" in
sources) and only then compute cc_idx = sources.index("claude_code") and yt_idx
= sources.index("youtube") and assert cc_idx < yt_idx to enforce the priority
ordering.

---

Duplicate comments:
In `@src/brainlayer/mcp/search_handler.py`:
- Around line 230-236: The entity-aware branch calls store.kg_hybrid_search
without forwarding the active filter parameters, which changes semantics
compared to the default _search path; update the store.kg_hybrid_search call in
search_handler.py (the block invoking store.kg_hybrid_search with
query_embedding, query_text/query, n_results/num_results, entity_name,
project_filter/normalized_project) to pass the current filter arguments
(content_type, source, tag, intent, importance_min, date_from, date_to,
sentiment) so that kg_hybrid_search receives the same filters as _search and
preserves query semantics.

In `@tests/test_groq_rate_limit.py`:
- Around line 53-69: The test currently only asserts the environment variable
string instead of the module constant; modify test_rate_limit_delay_env_override
to set os.environ["BRAINLAYER_GROQ_RATE_DELAY"]="5.0", reload the enrichment
module (e.g. import importlib; importlib.reload(brainlayer.pipeline.enrichment)
or import the module after setting the env), then assert that
brainlayer.pipeline.enrichment.GROQ_RATE_LIMIT_DELAY == 5.0 (or float(...)
equals 5.0); ensure you restore the original env var in the finally block as
before.

In `@tests/test_search_quality.py`:
- Around line 360-394: The test currently calls kg_hybrid_search directly so
routing logic in _brain_search is not exercised; modify the test to call
brainlayer.mcp.search_handler._brain_search (or the public entry that triggers
it) with the same inputs (query_text and query_embedding) and use a spy/patch on
the Store.kg_hybrid_search method (reference symbol: kg_hybrid_search) to assert
it was invoked and returned the expected results; ensure the existing DB setup
(kg_entities, kg_entities_fts, kg_entity_chunks, and _insert_chunk) remains the
same and replace the direct call to store.kg_hybrid_search with a call to
_brain_search, then assert the spy was called and that the returned payload
includes "chunks" and "facts".

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 819fd95 and 1655c89.

📒 Files selected for processing (3)
  • src/brainlayer/mcp/search_handler.py
  • tests/test_groq_rate_limit.py
  • tests/test_search_quality.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). (2)
  • GitHub Check: test (3.12)
  • GitHub Check: test (3.11)
🧰 Additional context used
📓 Path-based instructions (4)
**/*test*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Run tests with pytest

Files:

  • tests/test_groq_rate_limit.py
  • tests/test_search_quality.py
src/brainlayer/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use Python package structure with Typer CLI located in src/brainlayer/

Files:

  • src/brainlayer/mcp/search_handler.py
**/mcp/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Implement MCP tools: brain_search, brain_store, brain_recall, brain_entity, brain_expand, brain_update, brain_digest, brain_get_person with legacy brainlayer_* aliases

Files:

  • src/brainlayer/mcp/search_handler.py
src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Lint and format Python code using ruff check src/ and ruff format src/

Files:

  • src/brainlayer/mcp/search_handler.py
🧠 Learnings (2)
📚 Learning: 2026-03-02T21:14:04.731Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T21:14:04.731Z
Learning: Applies to **/enrichment*.py : Auto-switch enrichment backend from MLX to Ollama after 3 consecutive MLX failures; allow override with `BRAINLAYER_ENRICH_BACKEND` environment variable

Applied to files:

  • tests/test_groq_rate_limit.py
📚 Learning: 2026-03-02T21:14:04.731Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-02T21:14:04.731Z
Learning: Applies to **/mcp/**/*.py : Implement MCP tools: `brain_search`, `brain_store`, `brain_recall`, `brain_entity`, `brain_expand`, `brain_update`, `brain_digest`, `brain_get_person` with legacy `brainlayer_*` aliases

Applied to files:

  • src/brainlayer/mcp/search_handler.py
🧬 Code graph analysis (2)
tests/test_groq_rate_limit.py (2)
src/brainlayer/pipeline/enrichment.py (1)
  • call_groq (525-581)
src/brainlayer/session_repo.py (1)
  • get_unenriched_chunks (21-86)
tests/test_search_quality.py (4)
src/brainlayer/vector_store.py (1)
  • close (689-698)
src/brainlayer/mcp/search_handler.py (3)
  • _embed (726-727)
  • _embed (761-762)
  • _detect_entities (29-102)
src/brainlayer/search_repo.py (1)
  • hybrid_search (335-580)
src/brainlayer/kg_repo.py (1)
  • kg_hybrid_search (769-818)

Comment thread tests/test_groq_rate_limit.py
Comment thread tests/test_groq_rate_limit.py Outdated
EtanHey and others added 2 commits March 3, 2026 12:50
…rd, tests

- Rate limiter: wrap _groq_last_call check/update in threading.Lock
- Entity routing: skip kg_hybrid_search when active filters present
  (kg_hybrid_search doesn't support content_type/source/tag/etc.)
- Test: env-override now reloads module and asserts constant value
- Test: renamed misleading test, added filter-guard coverage

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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