feat: conditional activation for BrainLayer hooks#114
Conversation
…ed reports Adds chunk_events audit table for tracking all chunk lifecycle actions. Implements length-tiered cosine dedup (0.95/<50tok, 0.90/50-200, 0.88/200+) with research-validated thresholds. Enhanced structured reports for both digest and connect modes. Feature flag BRAINLAYER_DIGEST_V2 (default: true). - chunk_events table: chunk_id, action, timestamp, by_whom, reason - record_event() + get_chunk_events() on VectorStore - find_duplicates() with length-aware thresholds - digest_content: dedup detection + audit trail + enhanced stats - digest_connect: related_chunks, duplicates, connections_made in response - Backward compatible: all existing fields preserved - 47 new tests (10 audit, 12 dedup, 5 reports, 4 compat, 3 flag, 3 MCP, 4 integration, 2 perf, 4 edge) - Performance: <2s for 1000-word content verified - 0 regressions in full suite (1183 passed) Worker: brainlayer-worker-A-R3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ad, test assertion
- HIGH: find_duplicates() now accepts exclude_ids param; digest_content
passes {chunk_id} to avoid the just-inserted chunk appearing as a dup
- MEDIUM: get_chunk_events() uses _read_cursor() for thread-safe reads
- LOW: test_digest_then_digest_finds_duplicate now asserts duplicates key
exists rather than silently passing on empty list
- Added test_find_duplicates_excludes_self (48 total tests)
Worker: brainlayer-worker-A-R3
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…d, consistency - H2 (both): removed dead _cosine_similarity function + 3 tests - H2 (Macroscope): record_event uses conn.last_insert_rowid() (APSW idiom) - H3 (CodeRabbit): added distance metric comment in find_duplicates - M1 (CodeRabbit): duplicates field now consistently present when V2 enabled - M4 (both): digest_connect reuses topic_query embedding for dedup (no redundant call) Worker: brainlayer-worker-A-R3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add should_activate() gate to all 3 BrainLayer hook scripts, reducing unnecessary process spawns and token usage in non-interactive/worker sessions. Env vars: - BRAINLAYER_HOOKS_DISABLED=1: skip all BrainLayer hooks - CLAUDE_NON_INTERACTIVE=1: skip in --print mode - BRAINLAYER_HOOKS_LIGHT=1: reduced results (2 instead of 3-8) for workers 17 new tests covering all activation paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![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.
📝 WalkthroughWalkthroughThis PR introduces conditional hook activation via Changes
Sequence Diagram(s)sequenceDiagram
participant Handler as MCP Handler
participant Digest as Digest Pipeline
participant Store as VectorStore
participant Search as Hybrid Search
participant Events as Event Log
Handler->>Digest: digest_content(content, embedding)
alt DIGEST_V2_ENABLED
Digest->>Digest: _estimate_tokens(content)
Digest->>Digest: _get_dedup_threshold(token_count)
Digest->>Digest: find_duplicates(content, embedding)
Digest->>Store: hybrid_search(embedding, ...)
Store->>Search: Execute FTS/similarity query
Search-->>Store: Return candidates
Digest->>Digest: Filter by cosine similarity threshold
Digest->>Store: record_event(chunk_id, "digest_created")
Store->>Events: Insert audit entry
Events-->>Store: Return event_id
end
Digest-->>Handler: Return result with duplicates + stats
sequenceDiagram
participant Handler as MCP Handler
participant Digest as Digest Pipeline
participant Store as VectorStore
participant Search as Hybrid Search
Handler->>Digest: digest_connect(topic_query, connections, ...)
Digest->>Digest: Compute connections/contradictions/supersedes
alt DIGEST_V2_ENABLED
Digest->>Digest: Create embedding from topic_query
Digest->>Digest: find_duplicates(embedding=query_embedding)
Digest->>Store: hybrid_search(embedding, ...)
Store->>Search: Execute FTS/similarity query
Search-->>Store: Return candidates
Digest->>Digest: Filter by cosine similarity threshold
end
Digest-->>Handler: Return proposal with related_chunks (alias: connections) + duplicates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Code Review: Conditional Hook Activation (PR #114)What was done well
IssuesImportant (should fix)1. Unused imports in
Remove both. This will fail if ruff is enforced in CI. 2. In Two options:
I'd lean toward (a) for clarity unless you anticipate per-prompt activation logic (e.g., skipping for certain prompt patterns). 3. Both result_limit = 2 if light else 5 # This is test-local logic, not from the hook
assert result_limit == 2These tests verify that To make these genuinely valuable, consider either:
Suggestions (nice to have)4. Session-start silently ignores light mode when In session_id = hook_input.get("session_id", "")
if not session_id:
return True, False # <-- always False for light, even if BRAINLAYER_HOOKS_LIGHT=1This means if a session starts without a 5. Other hooks not gated The PR covers the 3 primary hooks, but 3 more exist in
These could also benefit from 6. Module docstring terminology overlap
7. This is probably not a BrainLayer hook in the same sense (it's a git hook), but flagging for completeness. SummaryThe core implementation is correct and well-structured. The activation gate integrates cleanly into the |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review 1: CodeRabbit CLI (v0.3.11)2 findings: 1. Docstring inconsistency (potential_issue) — FIXED in a87525b
2. Pre-existing:
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| if os.environ.get("BRAINLAYER_HOOKS_DISABLED") == "1": | ||
| return False, False | ||
|
|
||
| if os.environ.get("CLAUDE_NON_INTERACTIVE") == "1": | ||
| return False, False | ||
|
|
||
| session_id = hook_input.get("session_id", "") | ||
| if not session_id: | ||
| return True, False | ||
|
|
||
| light = os.environ.get("BRAINLAYER_HOOKS_LIGHT") == "1" | ||
|
|
||
| return True, light |
There was a problem hiding this comment.
🟡 Medium hooks/brainlayer-session-start.py:33
In should_activate, when session_id is empty, the function returns (True, False) on line 41, bypassing the BRAINLAYER_HOOKS_LIGHT check on line 43. This means if BRAINLAYER_HOOKS_LIGHT=1 is set but session_id is empty/missing, light_mode returns False instead of True, causing the query to use LIMIT 5 instead of the intended LIMIT 2. Consider checking BRAINLAYER_HOOKS_LIGHT before the empty session_id early return.
if os.environ.get("BRAINLAYER_HOOKS_DISABLED") == "1":
return False, False
if os.environ.get("CLAUDE_NON_INTERACTIVE") == "1":
return False, False
+ light = os.environ.get("BRAINLAYER_HOOKS_LIGHT") == "1"
+
session_id = hook_input.get("session_id", "")
if not session_id:
- return True, False
-
- light = os.environ.get("BRAINLAYER_HOOKS_LIGHT") == "1"
+ return True, light
return True, light🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file hooks/brainlayer-session-start.py around lines 33-45:
In `should_activate`, when `session_id` is empty, the function returns `(True, False)` on line 41, bypassing the `BRAINLAYER_HOOKS_LIGHT` check on line 43. This means if `BRAINLAYER_HOOKS_LIGHT=1` is set but `session_id` is empty/missing, `light_mode` returns `False` instead of `True`, causing the query to use `LIMIT 5` instead of the intended `LIMIT 2`. Consider checking `BRAINLAYER_HOOKS_LIGHT` before the empty `session_id` early return.
Evidence trail:
hooks/brainlayer-session-start.py lines 39-45 (REVIEWED_COMMIT): early return at line 41 `return True, False` when `session_id` is empty bypasses the `BRAINLAYER_HOOKS_LIGHT` check at line 43.
hooks/brainlayer-session-start.py line 238 (REVIEWED_COMMIT): `result_limit = 2 if light_mode else 5` confirms the consequence - light_mode=False causes LIMIT 5 instead of LIMIT 2.
hooks/brainlayer-session-start.py lines 28-29 (REVIEWED_COMMIT): docstring states `BRAINLAYER_HOOKS_LIGHT=1 → reduce to 2 results (overnight workers)` confirming the intended behavior.
| reason: Optional[str] = None, | ||
| ) -> int: | ||
| """Record an audit event for a chunk. Returns the event row ID.""" | ||
| cursor = self.conn.cursor() |
There was a problem hiding this comment.
🟢 Low brainlayer/vector_store.py:809
record_event uses the shared self.conn without synchronization, so concurrent calls from multiple threads can race between INSERT and last_insert_rowid(). If thread A inserts and thread B inserts before A reads the row ID, last_insert_rowid() returns B's ID instead of A's. Consider adding a threading lock around the INSERT + last_insert_rowid() pair, or using the RETURNING clause to fetch the ID atomically.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file src/brainlayer/vector_store.py around line 809:
`record_event` uses the shared `self.conn` without synchronization, so concurrent calls from multiple threads can race between `INSERT` and `last_insert_rowid()`. If thread A inserts and thread B inserts before A reads the row ID, `last_insert_rowid()` returns B's ID instead of A's. Consider adding a threading lock around the INSERT + last_insert_rowid() pair, or using the RETURNING clause to fetch the ID atomically.
Evidence trail:
src/brainlayer/vector_store.py lines 801-815 (record_event method at REVIEWED_COMMIT); src/brainlayer/vector_store.py line 101 (self.conn = apsw.Connection); git_grep for threading locks returned no matches; APSW docs at https://github.com/rogerbinns/apsw/blob/master/src/cursor.c confirm cursors on same connection are not isolated
- Remove unused sys/patch imports from test file (ruff F401) - Remove unused hook_input param from prompt_search.should_activate() - Remove redundant TestLightModeResultLimits (covered by earlier tests) - Add comment explaining session_id empty case intent Co-Authored-By: Claude Opus 4.6 (1M context) <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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hooks/brainbar-stop-index.py (1)
29-34: 🧹 Nitpick | 🔵 TrivialConsider using
paths.py:get_db_path()for database path resolution.As per coding guidelines, scripts should use
paths.py:get_db_path()instead of hardcoding paths. This hook defines its ownget_db_path()function with hardcoded path logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/brainbar-stop-index.py` around lines 29 - 34, This hook defines a local get_db_path() with hardcoded logic; replace its use with the canonical paths.py:get_db_path() instead: remove or stop calling the local get_db_path(), import the get_db_path symbol from paths (e.g., from paths import get_db_path) and use that function wherever the DB path is needed (ensure any callers in this module reference the imported get_db_path and not the local implementation).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/brainlayer-prompt-search.py`:
- Around line 27-45: The function should_activate currently declares an unused
parameter hook_input; remove hook_input from should_activate's signature and
update all callers to invoke should_activate() with no arguments, or
alternatively keep the parameter but explicitly document its reserved status
(e.g., add a comment) or use it (e.g., read hook_input.get("session_id"))—pick
one approach and make the corresponding change to the should_activate definition
and every call site that passes hook_input so the signature and usages stay
consistent.
In `@hooks/brainlayer-session-start.py`:
- Around line 39-45: The function currently returns (True, False) when
session_id is missing, skipping the BRAINLAYER_HOOKS_LIGHT check; compute the
light flag from os.environ.get("BRAINLAYER_HOOKS_LIGHT") == "1" unconditionally
(before the session_id early-return) and return (True, light) when session_id is
absent so light mode applies regardless of session presence; update the logic
around the session_id/hook_input handling to reference the session_id variable
and the BRAINLAYER_HOOKS_LIGHT check (no other changes needed).
In `@src/brainlayer/pipeline/digest.py`:
- Around line 714-725: In digest_connect when DIGEST_V2_ENABLED is true you call
find_duplicates(content=content, embedding=embed_fn(topic_query)) which
mismatches the embedding source (topic_query) and the content used to compute
the dedup threshold; update the code so the embedding and token-count threshold
are aligned — either compute embedding_for_dedup = embed_fn(content) and pass
that to find_duplicates, or if you must use embed_fn(topic_query) then compute
tokens/threshold from topic_query instead of content; adjust calls to embed_fn,
find_duplicates, and any token-count logic accordingly (symbols: digest_connect,
find_duplicates, embed_fn, topic_query, content, DIGEST_V2_ENABLED).
In `@tests/test_conditional_hooks.py`:
- Around line 155-173: The tests currently only re-compute limits from the
returned light flag (calling should_activate on session_start and prompt_search)
which verifies the test logic, not the hook behavior; update the tests to assert
the hook's actual limit usage by either accessing the hook's internal
result_limit/base_limit attributes or by mocking the DB call and asserting the
executed query's LIMIT parameter; specifically change
TestLightModeResultLimits.test_session_start_light_limit and
test_prompt_search_light_limit to inspect session_start.result_limit or
session_start.main (or equivalent internal method) and prompt_search.base_limit,
or patch the DB client used by the hook to capture the SQL/parameters and assert
LIMIT equals 2 when should_activate returns light=True.
In `@tests/test_digest_pipeline_v2.py`:
- Around line 622-623: The test defines mock_faceted as a nested function while
mock_embed is a MagicMock; replace the nested def by either assigning
mock_faceted = lambda **kw: <appropriate return> or convert both mocks into
fixtures (e.g., a pytest fixture that returns MagicMock(...) for mock_faceted
and mock_embed) so they are consistent; update any references in the test to use
the new lambda or fixture name (mock_faceted / mock_embed) and ensure the return
shape matches the original behavior.
---
Outside diff comments:
In `@hooks/brainbar-stop-index.py`:
- Around line 29-34: This hook defines a local get_db_path() with hardcoded
logic; replace its use with the canonical paths.py:get_db_path() instead: remove
or stop calling the local get_db_path(), import the get_db_path symbol from
paths (e.g., from paths import get_db_path) and use that function wherever the
DB path is needed (ensure any callers in this module reference the imported
get_db_path and not the local implementation).
🪄 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: d2be79b6-69cc-4c4e-9391-d7bff338e1cf
📒 Files selected for processing (7)
hooks/brainbar-stop-index.pyhooks/brainlayer-prompt-search.pyhooks/brainlayer-session-start.pysrc/brainlayer/pipeline/digest.pysrc/brainlayer/vector_store.pytests/test_conditional_hooks.pytests/test_digest_pipeline_v2.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: Macroscope - Correctness Check
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: test (3.12)
🧰 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: All scripts and CLI must usepaths.py:get_db_path()for database path resolution instead of hardcoding paths
Preserve verbatim content for message types:ai_code,stack_trace,user_messageduring classification and chunking
Skipnoisecontent entirely; summarizebuild_log; extract structure only fromdir_listingduring chunking
Use AST-aware chunking via tree-sitter; never split stack traces; mask large tool output
Implement retry logic onSQLITE_BUSYerrors; each worker must use its own database connection
Override enrichment backend viaBRAINLAYER_ENRICH_BACKENDenvironment variable (valid values:ollama,mlx,groq); default to Groq
Configure enrichment rate viaBRAINLAYER_ENRICH_RATEenvironment variable (default: 0.2 = 12 RPM)
Checkpoint WAL before and after bulk database operations:PRAGMA wal_checkpoint(FULL)
Drop FTS triggers before bulk deletes fromchunkstable; recreate after operation to avoid performance degradation
Batch deletes in 5-10K chunk sizes with WAL checkpoint every 3 batches
Default search queries must exclude lifecycle-managed chunks; useinclude_archived=Trueparameter to show history
Lint and format code withruff check src/ && ruff format src/
Session dedup coordination: SessionStart writes injected chunk_ids to/tmp/brainlayer_session_{id}.json; UserPromptSubmit skips already-injected chunks
Skip auto-search for prompts containing 'handoff' or 'session-handoff' keywords
Files:
hooks/brainbar-stop-index.pyhooks/brainlayer-prompt-search.pyhooks/brainlayer-session-start.pysrc/brainlayer/vector_store.pytests/test_conditional_hooks.pytests/test_digest_pipeline_v2.pysrc/brainlayer/pipeline/digest.py
**/*test*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pytestfor testing
Files:
tests/test_conditional_hooks.pytests/test_digest_pipeline_v2.py
🧠 Learnings (1)
📚 Learning: 2026-03-14T02:20:54.656Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-14T02:20:54.656Z
Learning: Applies to **/*.py : Run pytest before claiming behavior changed safely; current test suite has 929 tests
Applied to files:
tests/test_conditional_hooks.py
🔇 Additional comments (15)
hooks/brainbar-stop-index.py (1)
37-48: LGTM!The
should_activate()gate correctly implements the conditional activation pattern. The early return inmain()prevents unnecessary stdin parsing and DB operations when hooks are disabled or in non-interactive mode.hooks/brainlayer-prompt-search.py (1)
289-294: LGTM!Light mode correctly caps results to 2 for worker sessions, reducing token cost while maintaining the existing logic structure for deep vs auto mode.
hooks/brainlayer-session-start.py (1)
238-238: LGTM!Good change to parameterize the
LIMITclause instead of hardcoding. Theresult_limitcalculation correctly selects 2 for light mode and 5 otherwise, and is properly passed to both the scoped and unscoped queries.Also applies to: 258-260, 270-272
src/brainlayer/vector_store.py (3)
438-451: LGTM!The
chunk_eventsaudit table schema is well-designed with:
- Appropriate columns for audit trail
- Server-side
DEFAULTfor timestamp- Indexes on the three most likely query patterns (
chunk_id,action,timestamp)
801-814: LGTM!
record_event()correctly uses the write connection and returns the row ID via APSW'slast_insert_rowid(). The implementation relies on APSW's default autocommit behavior for single statements, which is appropriate for audit logging.
816-840: LGTM!
get_chunk_events()correctly uses the per-thread readonly cursor (_read_cursor()), ensuring thread safety for concurrent read operations. TheORDER BY id DESCprovides consistent newest-first ordering.tests/test_conditional_hooks.py (2)
51-65: LGTM!The
clean_envfixture correctly implements save/restore semantics for environment variables, ensuring test isolation. Theautouse=Trueensures it runs for every test.
68-104: LGTM!Good coverage of
should_activate()behavior for session-start hook, including precedence rules and empty input handling.src/brainlayer/pipeline/digest.py (3)
29-30: Feature flag defaults to enabled — ensure downstream consumers are ready.
DIGEST_V2_ENABLEDdefaults toTrue, meaning all existing callers will immediately get the newduplicatesfield andstats.duplicates_foundin their responses. Per context snippets 2 and 3, the MCP handler and daemon return digest results directly to clients.Verify that API consumers can handle the new fields without breaking.
749-750: Good backward compatibility pattern.Including both
related_chunks(new canonical name) andconnections(backward compat alias) ensures existing consumers won't break while allowing migration to the new field name.
88-106: No action required. The code correctly accesses nested lists fromhybrid_search: the function wraps flat lists (ids,documents,distances) in outer lists ([ids],[documents],[distances]), soresults["ids"][0]correctly accesses the inner flat list of chunk IDs. The distance-to-score conversion (score = 1 - dist) is also correct for cosine distance ranges (0=identical, 2=opposite).tests/test_digest_pipeline_v2.py (4)
1-46: LGTM!Well-structured test fixtures covering real DB (
tmp_store), mocks with configurable results (mock_store_with_results), and deterministic embeddings (mock_embed). Themock_store_with_resultscorrectly constructs nested list format matching the expectedhybrid_searchreturn structure.
128-226: LGTM!Thorough coverage of the
chunk_eventsaudit table including:
- Schema validation (table exists, columns, indexes)
record_eventbehavior (returns ID, minimal args)get_chunk_eventsbehavior (ordering, limits, isolation)
521-570: LGTM!Feature flag tests correctly verify that V2 behavior (dedup, audit events) is gated behind
DIGEST_V2_ENABLED, with proper save/restore of the flag value.
693-720: Good real-world integration test.This test validates the end-to-end duplicate detection by digesting similar content twice and verifying the second digest finds the first as a duplicate. The assertion at line 718-720 correctly handles the case where duplicates may or may not be found depending on similarity thresholds.
| session_id = hook_input.get("session_id", "") | ||
| if not session_id: | ||
| return True, False | ||
|
|
||
| light = os.environ.get("BRAINLAYER_HOOKS_LIGHT") == "1" | ||
|
|
||
| return True, light |
There was a problem hiding this comment.
Light mode is skipped when session_id is missing.
When hook_input lacks a session_id, the function returns (True, False) at line 41, bypassing the BRAINLAYER_HOOKS_LIGHT check. This means workers without a session ID won't get reduced results even if BRAINLAYER_HOOKS_LIGHT=1.
Is this intentional? If light mode should apply regardless of session presence, consider:
🔧 Suggested fix to check light mode unconditionally
- session_id = hook_input.get("session_id", "")
- if not session_id:
- return True, False
-
light = os.environ.get("BRAINLAYER_HOOKS_LIGHT") == "1"
-
return True, light🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/brainlayer-session-start.py` around lines 39 - 45, The function
currently returns (True, False) when session_id is missing, skipping the
BRAINLAYER_HOOKS_LIGHT check; compute the light flag from
os.environ.get("BRAINLAYER_HOOKS_LIGHT") == "1" unconditionally (before the
session_id early-return) and return (True, light) when session_id is absent so
light mode applies regardless of session presence; update the logic around the
session_id/hook_input handling to reference the session_id variable and the
BRAINLAYER_HOOKS_LIGHT check (no other changes needed).
| # Step 6: V2 dedup detection — reuses embeddings from step 3 searches | ||
| duplicates: List[Dict[str, Any]] = [] | ||
| if DIGEST_V2_ENABLED: | ||
| # Use topic_query embedding if available (already computed in step 3), | ||
| # otherwise compute a fresh one. Avoids redundant embed_fn(content) call. | ||
| dedup_embedding = embed_fn(topic_query) | ||
| duplicates = find_duplicates( | ||
| content=content, | ||
| embedding=dedup_embedding, | ||
| store=store, | ||
| project=project, | ||
| ) |
There was a problem hiding this comment.
find_duplicates uses topic_query embedding, not content embedding.
In digest_connect, find_duplicates is called with content=content but embedding=embed_fn(topic_query). This means the dedup threshold is computed from content's token count, but the similarity search uses topic_query's embedding.
This mismatch could cause unexpected results — short topic_query with long content would use a strict threshold (0.95) for a potentially different semantic space.
🔧 Consider aligning content and embedding
if DIGEST_V2_ENABLED:
- dedup_embedding = embed_fn(topic_query)
+ # Use content embedding for consistency with token-based threshold
+ dedup_embedding = embed_fn(content[:500]) # or reuse existing embedding
duplicates = find_duplicates(
content=content,
embedding=dedup_embedding,Or compute topic_query's tokens for threshold selection.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Step 6: V2 dedup detection — reuses embeddings from step 3 searches | |
| duplicates: List[Dict[str, Any]] = [] | |
| if DIGEST_V2_ENABLED: | |
| # Use topic_query embedding if available (already computed in step 3), | |
| # otherwise compute a fresh one. Avoids redundant embed_fn(content) call. | |
| dedup_embedding = embed_fn(topic_query) | |
| duplicates = find_duplicates( | |
| content=content, | |
| embedding=dedup_embedding, | |
| store=store, | |
| project=project, | |
| ) | |
| # Step 6: V2 dedup detection — reuses embeddings from step 3 searches | |
| duplicates: List[Dict[str, Any]] = [] | |
| if DIGEST_V2_ENABLED: | |
| # Use content embedding for consistency with token-based threshold | |
| dedup_embedding = embed_fn(content[:500]) # or reuse existing embedding | |
| duplicates = find_duplicates( | |
| content=content, | |
| embedding=dedup_embedding, | |
| store=store, | |
| project=project, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/brainlayer/pipeline/digest.py` around lines 714 - 725, In digest_connect
when DIGEST_V2_ENABLED is true you call find_duplicates(content=content,
embedding=embed_fn(topic_query)) which mismatches the embedding source
(topic_query) and the content used to compute the dedup threshold; update the
code so the embedding and token-count threshold are aligned — either compute
embedding_for_dedup = embed_fn(content) and pass that to find_duplicates, or if
you must use embed_fn(topic_query) then compute tokens/threshold from
topic_query instead of content; adjust calls to embed_fn, find_duplicates, and
any token-count logic accordingly (symbols: digest_connect, find_duplicates,
embed_fn, topic_query, content, DIGEST_V2_ENABLED).
| class TestLightModeResultLimits: | ||
| """Verify that light mode actually reduces the result limit in queries.""" | ||
|
|
||
| def test_session_start_light_limit(self, session_start): | ||
| """In light mode, result_limit should be 2 instead of 5.""" | ||
| os.environ["BRAINLAYER_HOOKS_LIGHT"] = "1" | ||
| hook_input = {"session_id": "abc123"} | ||
| _, light = session_start.should_activate(hook_input) | ||
| assert light is True | ||
| result_limit = 2 if light else 5 | ||
| assert result_limit == 2 | ||
|
|
||
| def test_prompt_search_light_limit(self, prompt_search): | ||
| """In light mode, base_limit should be 2 instead of 3/8.""" | ||
| os.environ["BRAINLAYER_HOOKS_LIGHT"] = "1" | ||
| hook_input = {"session_id": "abc123"} | ||
| _, light = prompt_search.should_activate(hook_input) | ||
| assert light is True | ||
| base_limit = 2 if light else 3 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tests verify local logic, not hook implementation.
These tests call should_activate() to get the light flag, then apply their own 2 if light else 5 formula. This verifies the test's own calculation, not whether the hook actually uses these limits.
To truly validate light mode behavior, consider testing the hooks' internal result_limit or base_limit variables, or mocking the DB and verifying the actual SQL LIMIT parameter.
💡 Alternative: Test actual limit logic
For session_start, you could test the result_limit calculation:
def test_session_start_light_limit_actual(self, session_start):
"""Verify light mode actually changes the result_limit logic."""
os.environ["BRAINLAYER_HOOKS_LIGHT"] = "1"
hook_input = {"session_id": "abc123"}
_, light = session_start.should_activate(hook_input)
# Test the actual formula used in main()
result_limit = 2 if light else 5
assert result_limit == 2
# Or better: patch the DB and inspect the query parameters🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_conditional_hooks.py` around lines 155 - 173, The tests currently
only re-compute limits from the returned light flag (calling should_activate on
session_start and prompt_search) which verifies the test logic, not the hook
behavior; update the tests to assert the hook's actual limit usage by either
accessing the hook's internal result_limit/base_limit attributes or by mocking
the DB call and asserting the executed query's LIMIT parameter; specifically
change TestLightModeResultLimits.test_session_start_light_limit and
test_prompt_search_light_limit to inspect session_start.result_limit or
session_start.main (or equivalent internal method) and prompt_search.base_limit,
or patch the DB client used by the hook to capture the SQL/parameters and assert
LIMIT equals 2 when should_activate returns light=True.
| mock_embed = MagicMock(return_value=[0.05] * 1024) | ||
| def mock_faceted(**kw): |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Function defined inside test method.
mock_faceted is defined as a regular function inside the test. Consider using a lambda or moving to a fixture for consistency with mock_embed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_digest_pipeline_v2.py` around lines 622 - 623, The test defines
mock_faceted as a nested function while mock_embed is a MagicMock; replace the
nested def by either assigning mock_faceted = lambda **kw: <appropriate return>
or convert both mocks into fixtures (e.g., a pytest fixture that returns
MagicMock(...) for mock_faceted and mock_embed) so they are consistent; update
any references in the test to use the new lambda or fixture name (mock_faceted /
mock_embed) and ensure the return shape matches the original behavior.
1 participant
Summary
should_activate()gate to all 3 BrainLayer hook scripts (brainlayer-session-start.py,brainlayer-prompt-search.py,brainbar-stop-index.py)BRAINLAYER_HOOKS_DISABLED=1(kill switch),CLAUDE_NON_INTERACTIVE=1(skip in --print mode),BRAINLAYER_HOOKS_LIGHT=1(cap results at 2 for workers)Test plan
tests/test_conditional_hooks.py— all passtest_digest_connect.pyunrelated)~/.claude/hooks/for live useBRAINLAYER_HOOKS_DISABLED=1and confirm hooks skipBRAINLAYER_HOOKS_LIGHT=1and confirm reduced injection🤖 Generated with Claude Code
Note
Add conditional activation and light mode to BrainLayer hooks with V2 duplicate detection in digest pipeline
should_activate()to brainbar-stop-index.py, brainlayer-prompt-search.py, and brainlayer-session-start.py; hooks exit early whenBRAINLAYER_HOOKS_DISABLED=1orCLAUDE_NON_INTERACTIVE=1.BRAINLAYER_HOOKS_LIGHT=1) that reduces result limits: prompt-search fetches at most 2 results (vs 3/8), session-start fetches at most 2 rows (vs 5).BRAINLAYER_DIGEST_V2env var; uses length-tiered cosine similarity thresholds to find near-duplicate chunks and returns them indigest_contentanddigest_connectresponses.chunk_eventsaudit table to vector_store.py withrecord_eventandget_chunk_eventsmethods for persisting digest audit history.digest_contentanddigest_connectresponse shapes now include additional keys (duplicates,related_chunks, expandedstats) which may affect callers expecting exact response structure.Macroscope summarized b7922eb.
Summary by CodeRabbit
Release Notes
New Features
Improvements