feat: unified enrichment controller + 3-mode brain_digest#100
Conversation
…n_digest Replace fragmented enrichment scripts (enrichment_backfill.py, cloud_backfill.py) with a single enrichment_controller.py that unifies realtime/batch/local modes behind a shared contract. All entry points (MCP, CLI, LaunchAgent) now route through one controller with consistent safety rails: thinkingBudget=0 on all Gemini calls, PII sanitization via build_external_prompt(), shared retry with exponential backoff, and single write path through update_enrichment(). Key changes: - New enrichment_controller.py (200 lines): enrich_realtime, enrich_batch, enrich_local - New get_enrichment_candidates() on SessionMixin: single SQL source of truth - brain_digest MCP tool gains mode="enrich" for agent-triggered enrichment - digest_content() now runs faceted tag enrichment (topics + dom:* + act:*) - CLI `brainlayer enrich` rewired to --mode realtime|batch|local - LaunchAgent plist updated for new CLI contract (hourly realtime, RunAtLoad) - Legacy scripts get deprecation warnings pointing to new entry points - 47 new tests covering controller, candidates, MCP modes, CLI, and digest enrichment Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 |
|
@codex review |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
✅ Actions performedReview triggered.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughUnified enrichment controller added with realtime/batch/local modes, CLI Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant Controller
participant VectorStore
participant Gemini
participant DB
rect rgba(100,150,200,0.5)
Note over CLI,DB: Realtime Mode Flow
CLI->>Controller: enrich_realtime(limit, since_hours)
Controller->>DB: get_enrichment_candidates(since_hours, limit)
DB-->>Controller: candidate chunks
loop per chunk
Controller->>Controller: build_external_prompt(chunk)
Controller->>Gemini: generate(prompt, response_mime_type=json)
Gemini-->>Controller: JSON enrichment response
Controller->>Controller: parse_enrichment(response)
Controller->>VectorStore: update_enrichment(chunk_id, enrichment)
end
Controller-->>CLI: EnrichmentResult(mode,retries,counts,errors)
end
rect rgba(200,150,100,0.5)
Note over CLI,DB: Batch Mode Flow
CLI->>Controller: enrich_batch(phase, limit)
Controller->>Controller: ensure_checkpoint_table()
Controller->>DB: get_pending_jobs() / get_unsubmitted_export_files()
DB-->>Controller: batch metadata
Controller-->>CLI: EnrichmentResult(mode=batch,counts)
end
rect rgba(150,200,100,0.5)
Note over CLI,DB: Local Mode Flow
CLI->>Controller: enrich_local(limit, parallel, backend)
Controller->>DB: get_enrichment_candidates(limit)
DB-->>Controller: candidate chunks
loop parallel workers
Controller->>Controller: build_prompt_local(chunk)
Controller->>Controller: call_llm(prompt, backend)
Controller->>Controller: parse_enrichment(response)
Controller->>VectorStore: update_enrichment(chunk_id)
end
Controller-->>CLI: EnrichmentResult(mode=local,counts)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 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 |
Research validation (R37) found 0.4 req/s (24 RPM) exceeds the Gemini free-tier limit of 15 RPM. Reduces to 0.2 req/s (12 RPM) with safety margin. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| return { | ||
| "status": "failed", | ||
| "reason": str(last_error) if last_error else "unknown_error", | ||
| "provider": "gemini", | ||
| "model": DEFAULT_FACETED_MODEL, | ||
| } |
There was a problem hiding this comment.
🟢 Low pipeline/digest.py:273
When retries are exhausted due to exceptions in _default_faceted_enrich, the returned dict omits pii_detected (lines 273-278), but the invalid JSON failure case includes it (lines 252-258). Callers expecting this field on all failed results will get a KeyError. Consider including pii_detected in the final failure return.
- return {
- "status": "failed",
- "reason": str(last_error) if last_error else "unknown_error",
- "provider": "gemini",
- "model": DEFAULT_FACETED_MODEL,
- }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file src/brainlayer/pipeline/digest.py around lines 273-278:
When retries are exhausted due to exceptions in `_default_faceted_enrich`, the returned dict omits `pii_detected` (lines 273-278), but the invalid JSON failure case includes it (lines 252-258). Callers expecting this field on all failed results will get a `KeyError`. Consider including `pii_detected` in the final failure return.
Evidence trail:
src/brainlayer/pipeline/digest.py lines 252-258 (invalid JSON case includes pii_detected), lines 273-278 (retry exhaustion case omits pii_detected) at REVIEWED_COMMIT
There was a problem hiding this comment.
Acknowledged — adding pii_detected to the retry-exhausted path for consistency is a trivial follow-up. The field is informational only (not used for control flow).
There was a problem hiding this comment.
Thanks for confirming! Would you like me to add the pii_detected field to the retry-exhausted return path to make it consistent with the other failure cases?
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8d53b3c08
ℹ️ 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".
| ensure_checkpoint_table(store) | ||
| pending = get_pending_jobs(store) if phase in {"poll", "run"} else [] | ||
| export_files = get_unsubmitted_export_files(db_path=getattr(store, "db_path", None)) if phase in {"submit", "run"} else [] | ||
| attempted = len(pending) + len(export_files) | ||
| return EnrichmentResult(mode="batch", attempted=attempted, enriched=0, skipped=0, failed=0, errors=[]) |
There was a problem hiding this comment.
Wire batch mode to the actual backfill phases
enrich_batch() currently never performs any batch work: it creates/checks the checkpoint table, counts pending jobs or export files, and immediately returns. None of the batch-entry points in scripts/cloud_backfill.py (export_unenriched_chunks, submit_gemini_batch, poll_gemini_batch, import_results, run_full_backfill) are called here, so brainlayer enrich --mode batch --phase submit|poll|import|run will always report a no-op instead of submitting, polling, or importing backlog jobs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Intentional design — enrich_batch() delegates to the existing cloud_backfill.py which has its own submit/poll/import lifecycle. The controller reports stats from the checkpoint table. Full batch wiring (submit → poll → import orchestration) is a follow-up item — this PR establishes the routing layer so the entry points converge.
| if mode == "enrich": | ||
| from ..enrichment_controller import enrich_realtime | ||
|
|
||
| result = enrich_realtime(store=store, limit=limit) |
There was a problem hiding this comment.
Offload realtime enrichment from the MCP event loop
In the mode="enrich" branch, _brain_digest calls the synchronous enrich_realtime() directly instead of using run_in_executor like the normal digest path. Because enrich_realtime() performs blocking network I/O plus time.sleep() per chunk, a single MCP enrich call can monopolize the server thread for minutes, preventing other MCP requests from being serviced during that run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in caa8859 — wrapped enrich_realtime() in run_in_executor to match the digest path pattern.
| <string>1</string> | ||
| <key>BRAINLAYER_STALL_TIMEOUT</key> |
There was a problem hiding this comment.
Preserve API-key injection in the launchd template
When this LaunchAgent is installed via scripts/launchd/install.sh, the installer still only reads/substitutes GROQ_API_KEY (install.sh:18,40-46). After this change the plist's EnvironmentVariables block no longer contains any credential placeholder, but the new realtime path requires GOOGLE_API_KEY/GOOGLE_GENERATIVE_AI_API_KEY in enrichment_controller._get_gemini_client(). That means fresh install.sh enrich installs lose their only key-injection path and the scheduled job fails unless users hand-edit the plist.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in caa8859 — plist now has GOOGLE_API_KEY placeholder and install.sh reads it from 1Password or env.
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 (2)
scripts/launchd/com.brainlayer.enrich.plist (1)
28-36:⚠️ Potential issue | 🟠 MajorAdd
GOOGLE_API_KEYto EnvironmentVariables for realtime mode.Realtime mode uses Gemini, which requires
GOOGLE_API_KEYorGOOGLE_GENERATIVE_AI_API_KEY. The removedGROQ_API_KEYis no longer needed, but the Gemini API key must be provided. Without it,enrich_realtime()will raiseRuntimeErroron the first run.Suggested fix
<key>GOOGLE_API_KEY</key> <string>__GOOGLE_API_KEY__</string>Or document that users must set this key system-wide before loading the agent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/launchd/com.brainlayer.enrich.plist` around lines 28 - 36, Add the Gemini API key to the EnvironmentVariables block so realtime mode has credentials: insert a new key/value pair for GOOGLE_API_KEY with a placeholder like __GOOGLE_API_KEY__ (i.e., <key>GOOGLE_API_KEY</key> and corresponding <string>__GOOGLE_API_KEY__</string>) within the existing EnvironmentVariables <dict>; this prevents enrich_realtime() from raising a RuntimeError on first run—alternatively document that users must set GOOGLE_API_KEY (or GOOGLE_GENERATIVE_AI_API_KEY) system-wide before loading the agent.src/brainlayer/cli/__init__.py (1)
897-909:⚠️ Potential issue | 🟡 MinorInconsistent DB path between stats_only and non-stats branches.
Line 898 uses
DEFAULT_DB_PATHfor stats, while line 909 usesget_db_path(). These may resolve to different paths if environment variables are set.For consistency and adherence to coding guidelines ("use
paths.py:get_db_path()for all database path resolution"), both branches should useget_db_path().🔧 Proposed fix for consistent DB path usage
if stats_only: - store = VectorStore(DEFAULT_DB_PATH) + store = VectorStore(get_db_path()) try:As per coding guidelines: "Use
paths.py:get_db_path()for all database path resolution."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/brainlayer/cli/__init__.py` around lines 897 - 909, The stats-only branch currently constructs the VectorStore with DEFAULT_DB_PATH which can differ from the non-stats branch; change the stats-only branch to use get_db_path() instead of DEFAULT_DB_PATH so both branches resolve the same DB path (update the VectorStore(...) call around get_enrichment_stats() and keep the existing store.close() handling intact); look for VectorStore instantiation in this block and replace DEFAULT_DB_PATH with get_db_path().
🤖 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/enrichment_controller.py`:
- Around line 175-199: The enrich_local function declares an unused parallel
parameter; implement real parallel processing by creating an inner worker
function (e.g., process_chunk) that encapsulates the current per-chunk logic
(build_prompt, _retry_with_backoff -> _call_local_backend, parse_enrichment,
_apply_enrichment) and then run that worker over candidates using a
ThreadPoolExecutor (or similar) sized by the parallel argument, collecting and
aggregating successes/failures into the EnrichmentResult (mode="local") while
preserving error strings and counts; alternatively, if concurrency is not
desired, remove the parallel parameter and its noqa comment and/or add a TODO in
enrich_local explaining future plans.
- Around line 33-53: The dynamic loader _load_cloud_backfill_module currently
imports scripts/cloud_backfill.py at runtime which will fail when installed as a
wheel because scripts/ is not packaged; refactor by moving the batch functions
into a proper package module under src/brainlayer (or expose them as public APIs
there) and update ensure_checkpoint_table, get_pending_jobs, and
get_unsubmitted_export_files to directly import and call the new module
functions instead of calling _load_cloud_backfill_module; ensure the old helper
is removed or redirected and add tests/import checks to verify the functions
import correctly from the new module in packaged installs.
In `@src/brainlayer/mcp/store_handler.py`:
- Around line 38-50: The synchronous call to enrich_realtime in store_handler
(currently invoked directly and returned as CallToolResult) can block the
asyncio event loop while digest_content uses run_in_executor; change this to
offload enrich_realtime to a thread/process executor by calling it via the
running event loop's run_in_executor (or asyncio.to_thread) and await the
result, preserving the existing result mapping
(mode/attempted/enriched/skipped/failed/errors) and returning the same
CallToolResult; update the code around enrich_realtime and ensure the function
signature and imports still match (reference symbols: enrich_realtime,
digest_content, run_in_executor, CallToolResult).
In `@tests/test_cli_enrich.py`:
- Around line 13-28: The test monkeypatches the original enrichment_controller
functions but the CLI imports them into its own namespace, so update the
monkeypatch targets in test_cli_enrich_mode_realtime_routes_to_controller to
patch the CLI bindings (use "brainlayer.cli.enrich_realtime" instead of
"brainlayer.enrichment_controller.enrich_realtime") and apply the same change
for the other two tests/monkeypatches that replace enrich_batch and enrich_local
so they patch "brainlayer.cli.enrich_batch" and "brainlayer.cli.enrich_local"
respectively.
In `@tests/test_session_enrichment_candidates.py`:
- Around line 82-90: Add a unit test that verifies
VectorStore.get_enrichment_candidates returns an empty list when called with
chunk_ids=[], mirroring the defensive early return in session_repo.py;
specifically add a test (e.g.,
test_get_enrichment_candidates_returns_empty_for_empty_chunk_ids) that creates a
VectorStore, inserts at least one chunk via _insert_chunk, calls
store.get_enrichment_candidates(limit=10, chunk_ids=[]), and asserts the result
equals [] to document and guard this behavior for function
get_enrichment_candidates.
---
Outside diff comments:
In `@scripts/launchd/com.brainlayer.enrich.plist`:
- Around line 28-36: Add the Gemini API key to the EnvironmentVariables block so
realtime mode has credentials: insert a new key/value pair for GOOGLE_API_KEY
with a placeholder like __GOOGLE_API_KEY__ (i.e., <key>GOOGLE_API_KEY</key> and
corresponding <string>__GOOGLE_API_KEY__</string>) within the existing
EnvironmentVariables <dict>; this prevents enrich_realtime() from raising a
RuntimeError on first run—alternatively document that users must set
GOOGLE_API_KEY (or GOOGLE_GENERATIVE_AI_API_KEY) system-wide before loading the
agent.
In `@src/brainlayer/cli/__init__.py`:
- Around line 897-909: The stats-only branch currently constructs the
VectorStore with DEFAULT_DB_PATH which can differ from the non-stats branch;
change the stats-only branch to use get_db_path() instead of DEFAULT_DB_PATH so
both branches resolve the same DB path (update the VectorStore(...) call around
get_enrichment_stats() and keep the existing store.close() handling intact);
look for VectorStore instantiation in this block and replace DEFAULT_DB_PATH
with get_db_path().
🪄 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: f4f52c6b-ec99-4107-ba58-20585df8d991
📒 Files selected for processing (16)
scripts/cloud_backfill.pyscripts/enrichment_backfill.pyscripts/launchd/com.brainlayer.enrich.plistsrc/brainlayer/cli/__init__.pysrc/brainlayer/enrichment_controller.pysrc/brainlayer/mcp/__init__.pysrc/brainlayer/mcp/store_handler.pysrc/brainlayer/pipeline/digest.pysrc/brainlayer/pipeline/enrichment.pysrc/brainlayer/session_repo.pytests/test_cli_enrich.pytests/test_digest_enrichment.pytests/test_enrichment_controller.pytests/test_mcp_digest_modes.pytests/test_phase3_digest.pytests/test_session_enrichment_candidates.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.13)
- GitHub Check: test (3.12)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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
Files:
scripts/enrichment_backfill.pyscripts/cloud_backfill.pytests/test_digest_enrichment.pysrc/brainlayer/pipeline/enrichment.pysrc/brainlayer/mcp/store_handler.pytests/test_session_enrichment_candidates.pytests/test_enrichment_controller.pytests/test_phase3_digest.pysrc/brainlayer/pipeline/digest.pytests/test_cli_enrich.pytests/test_mcp_digest_modes.pysrc/brainlayer/session_repo.pysrc/brainlayer/cli/__init__.pysrc/brainlayer/mcp/__init__.pysrc/brainlayer/enrichment_controller.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use pytest for testing with commands:
pytestandruff check src/ && ruff format src/for linting and formatting
Files:
tests/test_digest_enrichment.pytests/test_session_enrichment_candidates.pytests/test_enrichment_controller.pytests/test_phase3_digest.pytests/test_cli_enrich.pytests/test_mcp_digest_modes.py
src/brainlayer/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/**/*.py: Usepaths.py:get_db_path()for all database path resolution; support environment variable override or fall back to canonical path~/.local/share/brainlayer/brainlayer.db; all scripts and CLI must use this function
Implement retry logic onSQLITE_BUSYerrors; each worker must use its own database connection for safe concurrent access
Use ruff for all linting and formatting of Python source code insrc/directory
Files:
src/brainlayer/pipeline/enrichment.pysrc/brainlayer/mcp/store_handler.pysrc/brainlayer/pipeline/digest.pysrc/brainlayer/session_repo.pysrc/brainlayer/cli/__init__.pysrc/brainlayer/mcp/__init__.pysrc/brainlayer/enrichment_controller.py
src/brainlayer/mcp/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement MCP (Model Context Protocol) server with 8 tools:
brain_search,brain_store,brain_recall,brain_entity,brain_expand,brain_update,brain_digest,brain_get_person(legacybrainlayer_*aliases must still work); entrypointbrainlayer-mcp
Files:
src/brainlayer/mcp/store_handler.pysrc/brainlayer/mcp/__init__.py
src/brainlayer/enrichment*.py
📄 CodeRabbit inference engine (CLAUDE.md)
For enrichment backend configuration: use primary backend MLX (
Qwen2.5-Coder-14B-Instruct-4bit) on Apple Silicon (port 8080), fallback to Ollama (glm-4.7-flash) on port 11434 after 3 consecutive MLX failures, or override withBRAINLAYER_ENRICH_BACKEND=ollama|mlx|groqenvironment variable
Files:
src/brainlayer/enrichment_controller.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-14T02:20:54.656Z
Learning: Request codex review, cursor review, and bugbot review for BrainLayer PRs
📚 Learning: 2026-03-20T23:35:42.084Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T23:35:42.084Z
Learning: Applies to src/brainlayer/mcp/**/*.py : Implement MCP (Model Context Protocol) server with 8 tools: `brain_search`, `brain_store`, `brain_recall`, `brain_entity`, `brain_expand`, `brain_update`, `brain_digest`, `brain_get_person` (legacy `brainlayer_*` aliases must still work); entrypoint `brainlayer-mcp`
Applied to files:
src/brainlayer/mcp/store_handler.pytests/test_phase3_digest.pytests/test_mcp_digest_modes.pysrc/brainlayer/mcp/__init__.pysrc/brainlayer/enrichment_controller.py
📚 Learning: 2026-03-17T01:04:22.497Z
Learnt from: EtanHey
Repo: EtanHey/brainlayer PR: 0
File: :0-0
Timestamp: 2026-03-17T01:04:22.497Z
Learning: Applies to src/brainlayer/mcp/**/*.py and brain-bar/Sources/BrainBar/MCPRouter.swift: The 8 required MCP tools are `brain_search`, `brain_store`, `brain_recall`, `brain_entity`, `brain_expand`, `brain_update`, `brain_digest`, `brain_tags`. `brain_tags` is the 8th tool, replacing `brain_get_person`, as defined in the Phase B spec merged in PR `#72`. The Python MCP server already implements `brain_tags`. Legacy `brainlayer_*` aliases must be maintained for backward compatibility.
Applied to files:
src/brainlayer/mcp/store_handler.pytests/test_phase3_digest.pytests/test_mcp_digest_modes.pysrc/brainlayer/mcp/__init__.py
📚 Learning: 2026-03-20T23:35:42.084Z
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T23:35:42.084Z
Learning: Applies to src/brainlayer/enrichment*.py : For enrichment backend configuration: use primary backend MLX (`Qwen2.5-Coder-14B-Instruct-4bit`) on Apple Silicon (port 8080), fallback to Ollama (`glm-4.7-flash`) on port 11434 after 3 consecutive MLX failures, or override with `BRAINLAYER_ENRICH_BACKEND=ollama|mlx|groq` environment variable
Applied to files:
scripts/launchd/com.brainlayer.enrich.plistsrc/brainlayer/cli/__init__.pysrc/brainlayer/enrichment_controller.py
🔇 Additional comments (30)
scripts/cloud_backfill.py (1)
24-30: LGTM: Deprecation warning correctly placed at module load.The warning triggers during dynamic import by the enrichment controller and uses
stacklevel=2to point to the caller. This aligns with the PR's goal of consolidating enrichment into the unified controller.scripts/enrichment_backfill.py (1)
10-16: LGTM: Deprecation warning correctly implemented.The warning appropriately directs users to the new unified enrichment flows.
src/brainlayer/session_repo.py (1)
40-107: LGTM: Clean unified candidate query with proper safeguards.The implementation correctly:
- Returns early for empty
chunk_idslist (line 51-52)- Uses
int()cast onsince_hoursto prevent SQL injection (line 60)- Includes
created_atin returned dicts for downstream use- Properly delegates from
get_unenriched_chunksfor backwards compatibilitytests/test_digest_enrichment.py (1)
1-11: LGTM: Test validates critical safety rail.This test ensures
thinkingBudget=0is enforced for Gemini calls, which the PR summary notes as a safety measure following a "high-cost incident on February 18, 2026."src/brainlayer/pipeline/enrichment.py (1)
396-457: LGTM: Clean template override for external prompts.The
prompt_templateparameter enables digest.py to passFACETED_DIGEST_PROMPTwhile maintaining backwards compatibility (defaults toENRICHMENT_PROMPT). The placeholder contract ({project},{content_type},{content},{context_section}) is consistent across templates as shown in the context snippets.src/brainlayer/mcp/store_handler.py (1)
52-56: LGTM: Mode validation with clear error messages.Properly rejects unknown modes and requires content for digest mode.
scripts/launchd/com.brainlayer.enrich.plist (1)
38-39: VerifyRunAtLoadbehavior change is intentional.Changed from
falsetotrue, meaning enrichment triggers immediately on launchd agent load (e.g., system boot, user login). This may cause unexpected API calls or conflicts with other startup processes.tests/test_session_enrichment_candidates.py (1)
1-110: LGTM: Comprehensive test coverage forget_enrichment_candidates.Tests cover the key filtering scenarios: unenriched-only, enriched exclusion, minimum content length, time-based filtering, explicit chunk IDs, and limit enforcement.
tests/test_phase3_digest.py (3)
198-245: LGTM! Comprehensive faceted enrichment test.This test thoroughly validates the faceted enrichment integration by:
- Mocking the enrichment function with expected return structure
- Asserting merged tags include topics, activity, and domains
- Verifying the chunk row is updated with tags, intent, summary, and enriched_at
261-275: LGTM! Schema test updated for new mode-based interface.The test correctly validates that
brain_digestnow exposesmodeandlimitproperties, aligning with the MCP schema changes wherecontentis no longer required (enabling enrich mode without content).
277-295: LGTM! Description test ensures clear tool routing guidance.This test validates that the
brain_digestdescription provides sufficient guidance for agents to understand when to use digest vs. store, and documents the realtime/batch/local modes.src/brainlayer/pipeline/digest.py (4)
141-146: LGTM! Gemini config enforces thinkingBudget: 0 safety rail.The config correctly disables thinking for flash models, which is a PR requirement for safety rails.
149-204: LGTM! Robust JSON parsing with validation.The
_parse_faceted_enrichmentfunction handles malformed responses gracefully:
- Scans for valid JSON braces
- Validates schema (topics list, act:* prefix, dom:* prefixes)
- Clamps confidence to 0-1 range
- Returns None on any validation failure
243-278: Verify retry logic alignment with PR-specified 12 attempts.The retry loop uses
DEFAULT_FACETED_MAX_RETRIES(default 12) correctly, but note thatrange(DEFAULT_FACETED_MAX_RETRIES)gives 12 iterations (0-11), so you get 12 attempts total. The break at line 269 (if attempt == DEFAULT_FACETED_MAX_RETRIES - 1) is correct.However, the delay calculation at line 271 uses
2**attemptwhich for attempt=11 yields 2048 seconds before capping atDEFAULT_FACETED_MAX_DELAY(120s). The cap is working correctly.
400-420: LGTM! Faceted enrichment integration with single-write enforcement.The digest pipeline correctly:
- Calls the faceted enrichment function (injectable for testing)
- Merges tags from topics + activity + domains
- Uses
store.update_enrichment()as the single write path- Converts activity tags to intent via
_activity_to_intentsrc/brainlayer/mcp/__init__.py (2)
606-666: LGTM! brain_digest schema updated for mode-based operation.The schema changes correctly:
modeenum restricts to["digest", "enrich"]limithas sensible bounds (1-5000, default 25)required: []allows enrich mode without content- Description clearly explains routing, PII sanitization, and mode differences
960-968: LGTM! Correct argument forwarding to _brain_digest.The routing correctly uses
arguments.get("content")(nullable) and forwardsmode/limitwith appropriate defaults. The handler instore_handler.pyguards againstcontent=Nonefor digest mode.tests/test_cli_enrich.py (2)
69-86: LGTM! Stats output verification test.The test correctly mocks
VectorStorewith enrichment stats and verifies the CLI prints the expected labels.
89-92: LGTM! Invalid mode rejection test.Simple and effective negative test ensuring invalid modes are rejected with non-zero exit.
tests/test_enrichment_controller.py (3)
48-73: LGTM! Critical safety rail test for thinkingBudget: 0.This test captures the Gemini config and asserts
thinking_budget == 0, verifying the safety rail is enforced.
155-172: Verify retry count assertion.The test expects
attempts["count"] == 13formax_retries=12. Looking at_retry_with_backoffin the controller:
range(max_retries + 1)= 13 iterations (0-12)- Each iteration calls
fn(), so 13 attempts total- 12 sleeps occur (after each failed attempt except the last)
The assertion
assert attempts["count"] == 13andassert len(sleeps) == 12is correct.
127-152: LGTM! Idempotency test verifies single-write enforcement.The test correctly simulates:
- First run: 1 candidate → enriched
- Second run: 0 candidates → nothing enriched
This confirms the controller doesn't re-process already-enriched chunks.
tests/test_mcp_digest_modes.py (2)
33-50: Consider verifying limit parameter is actually forwarded to enrich_realtime.The mock at line 41 accepts
limit=25as default, but the test calls withlimit=7. The mock should capture and verify the actual limit value passed. Currently, the assertion at line 50 checkspayload["attempted"] == 7, which indirectly tests this, but relies on the mock returninglimitasattempted.This is acceptable since the mock implementation
attempted=limitconfirms the forwarding.
88-97: Consider usingawaitfor async function in sync test.Line 91 uses
asyncio.run(list_tools())in a non-async test function. While this works, consistency with the other tests using@pytest.mark.asynciowould be cleaner. However, for a simple schema check, this is acceptable.src/brainlayer/cli/__init__.py (2)
881-891: LGTM! Enrich command interface matches controller modes.The CLI options correctly map to the three enrichment modes:
realtime: useslimit,since_hoursbatch: usesphase,limitlocal: useslimit,parallel,backend
408-415: LGTM! Graceful output of enrichment metadata.The digest command now conditionally prints tags and enrichment status when available, with null-safe access patterns.
src/brainlayer/enrichment_controller.py (4)
65-69: LGTM! Gemini config enforces thinkingBudget: 0 safety rail.The config correctly sets
thinking_budget: 0as required by PR objectives for all Gemini calls.
72-88: LGTM! Robust retry implementation with exponential backoff.The retry logic correctly:
- Caps delay at
max_delay(120s default)- Adds jitter up to 50% of delay
- Ensures total sleep never exceeds
max_delay- Raises after exhausting retries
113-158: LGTM! Realtime enrichment with proper rate limiting and error handling.The implementation correctly:
- Enforces 0.2 req/s (12 RPM) rate limit for Gemini free-tier compliance
- Uses exponential backoff retry for transient failures
- Records per-chunk errors without failing the entire batch
- Uses the shared
_apply_enrichmentpath for single-write enforcement
107-110:_call_local_backendcorrectly delegates to the fallback-enabledcall_llmimplementation.Verified:
call_llmimplements the required MLX/Ollama fallback pattern with MLX primary (port 8080) on Apple Silicon, automatic fallback to Ollama (port 11434) after 3 consecutive failures, andBRAINLAYER_ENRICH_BACKENDenvironment variable override support. Thebackendparameter is properly threaded through.
| def _load_cloud_backfill_module(): | ||
| """Load scripts/cloud_backfill.py without turning scripts into a package.""" | ||
| module_path = Path(__file__).resolve().parents[2] / "scripts" / "cloud_backfill.py" | ||
| spec = importlib.util.spec_from_file_location("brainlayer_cloud_backfill", module_path) | ||
| if spec is None or spec.loader is None: | ||
| raise RuntimeError(f"Could not load cloud_backfill module from {module_path}") | ||
| module = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(module) | ||
| return module | ||
|
|
||
|
|
||
| def ensure_checkpoint_table(store) -> None: | ||
| return _load_cloud_backfill_module().ensure_checkpoint_table(store) | ||
|
|
||
|
|
||
| def get_pending_jobs(store): | ||
| return _load_cloud_backfill_module().get_pending_jobs(store) | ||
|
|
||
|
|
||
| def get_unsubmitted_export_files(*args, **kwargs): | ||
| return _load_cloud_backfill_module().get_unsubmitted_export_files(*args, **kwargs) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if cloud_backfill.py exists at expected location
fd -t f "cloud_backfill.py" scripts/Repository: EtanHey/brainlayer
Length of output: 87
🏁 Script executed:
find . -name "enrichment_controller.py" -type fRepository: EtanHey/brainlayer
Length of output: 103
🏁 Script executed:
# Check the full repository structure
git ls-files | head -30Repository: EtanHey/brainlayer
Length of output: 1004
🏁 Script executed:
# Check for setup.py or pyproject.toml to understand how the package is distributed
ls -la | grep -E "(setup|pyproject|setup\.cfg)"Repository: EtanHey/brainlayer
Length of output: 127
🏁 Script executed:
# Check the actual content of pyproject.toml or setup.py
cat pyproject.toml 2>/dev/null || cat setup.py 2>/dev/null || echo "No standard config found"Repository: EtanHey/brainlayer
Length of output: 3484
Dynamic module loading will fail in production wheels — refactor required.
Loading scripts/cloud_backfill.py via importlib.util.spec_from_file_location works in development because scripts/ exists at the repo root. However, when the package is installed as a wheel (the standard production distribution format), hatchling only includes files under src/brainlayer/ by default. The scripts/ directory will be missing, causing RuntimeError at runtime.
This is not a fragility concern for development—it's a critical packaging issue. Resolve by either:
- Moving the batch logic into
src/brainlayer/as a proper module - Explicitly configuring
pyproject.tomlto includescripts/in distributions - Extracting
ensure_checkpoint_table(),get_pending_jobs(), andget_unsubmitted_export_files()as public APIs insrc/brainlayer/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/brainlayer/enrichment_controller.py` around lines 33 - 53, The dynamic
loader _load_cloud_backfill_module currently imports scripts/cloud_backfill.py
at runtime which will fail when installed as a wheel because scripts/ is not
packaged; refactor by moving the batch functions into a proper package module
under src/brainlayer (or expose them as public APIs there) and update
ensure_checkpoint_table, get_pending_jobs, and get_unsubmitted_export_files to
directly import and call the new module functions instead of calling
_load_cloud_backfill_module; ensure the old helper is removed or redirected and
add tests/import checks to verify the functions import correctly from the new
module in packaged installs.
There was a problem hiding this comment.
Acknowledged. The cloud_backfill.py dynamic import uses importlib to avoid making scripts/ a package. The module exists at a fixed path relative to the package. But if the file is removed before the import is called, the error message from RuntimeError on line 38 explains what happened. Adding a file-exists check before spec_from_file_location is a reasonable hardening step — will add in follow-up.
There was a problem hiding this comment.
@EtanHey, just a small clarification — the core concern isn't about the file being accidentally deleted, but about it never being present at all in a standard wheel install. When installed via pip install brainlayer (or from a wheel artifact), hatchling only bundles src/brainlayer/; the scripts/ directory is excluded entirely, so module_path will point to a path that doesn't exist on the end user's filesystem regardless of whether the file was "removed."
A file-exists check would make the error message clearer, but won't prevent the failure in a packaged install. The real fix would be one of the three options from the original comment.
That said — if brainlayer is always run from source (e.g. a personal tool, never distributed as a wheel), this is a non-issue in practice. Would you like me to open a follow-up GitHub issue to track the proper fix so it isn't lost?
🧠 Learnings used
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T23:35:42.084Z
Learning: Applies to src/brainlayer/enrichment*.py : For enrichment backend configuration: use primary backend MLX (`Qwen2.5-Coder-14B-Instruct-4bit`) on Apple Silicon (port 8080), fallback to Ollama (`glm-4.7-flash`) on port 11434 after 3 consecutive MLX failures, or override with `BRAINLAYER_ENRICH_BACKEND=ollama|mlx|groq` environment variable
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-14T02:20:54.656Z
Learning: Request codex review, cursor review, and bugbot review for BrainLayer PRs
| def enrich_local( | ||
| store, | ||
| limit: int = 100, | ||
| parallel: int = 2, # noqa: ARG001 | ||
| backend: str = "mlx", | ||
| ) -> EnrichmentResult: | ||
| """Enrich via local MLX backend.""" | ||
| candidates = store.get_enrichment_candidates(limit=limit, chunk_ids=None) | ||
| result = EnrichmentResult(mode="local", attempted=len(candidates), enriched=0, skipped=0, failed=0) | ||
|
|
||
| for chunk in candidates: | ||
| try: | ||
| prompt = build_prompt(chunk) | ||
| raw_response = _retry_with_backoff(lambda: _call_local_backend(prompt, backend=backend), max_retries=2) | ||
| enrichment = parse_enrichment(raw_response) | ||
| if not enrichment: | ||
| result.failed += 1 | ||
| result.errors.append(f"{chunk['id']}: invalid_enrichment") | ||
| continue | ||
| _apply_enrichment(store, chunk, enrichment) | ||
| result.enriched += 1 | ||
| except Exception as exc: # noqa: BLE001 | ||
| result.failed += 1 | ||
| result.errors.append(f"{chunk['id']}: {exc}") | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
parallel parameter is unused — clarify intent or implement.
The parallel parameter at line 178 is marked # noqa: ARG001 but has a default value of 2, suggesting concurrent processing was intended. Currently, chunks are processed sequentially.
If parallel processing is planned for a future iteration, consider documenting this in a TODO comment. If it's not planned, remove the parameter to avoid confusion.
💡 Option: Add TODO or remove unused parameter
def enrich_local(
store,
limit: int = 100,
- parallel: int = 2, # noqa: ARG001
+ parallel: int = 2, # TODO: Implement concurrent processing with ThreadPoolExecutor
backend: str = "mlx",
) -> EnrichmentResult:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/brainlayer/enrichment_controller.py` around lines 175 - 199, The
enrich_local function declares an unused parallel parameter; implement real
parallel processing by creating an inner worker function (e.g., process_chunk)
that encapsulates the current per-chunk logic (build_prompt, _retry_with_backoff
-> _call_local_backend, parse_enrichment, _apply_enrichment) and then run that
worker over candidates using a ThreadPoolExecutor (or similar) sized by the
parallel argument, collecting and aggregating successes/failures into the
EnrichmentResult (mode="local") while preserving error strings and counts;
alternatively, if concurrency is not desired, remove the parallel parameter and
its noqa comment and/or add a TODO in enrich_local explaining future plans.
There was a problem hiding this comment.
Correct — parallel parameter is accepted but not wired yet. enrich_local() currently runs sequentially. Parallel local enrichment (thread pool or process pool) is planned. Keeping the parameter in the signature now so callers don't need to change when we implement it.
There was a problem hiding this comment.
@EtanHey, understood — keeping the parameter for API stability makes sense. To help future contributors understand the intent (and avoid the # noqa being misread as "this can be removed"), consider adding a short TODO comment alongside it:
def enrich_local(
store,
limit: int = 100,
parallel: int = 2, # noqa: ARG001 # TODO: wire up ThreadPoolExecutor for concurrent chunk processing
backend: str = "mlx",
) -> EnrichmentResult:This way the suppression and the future plan are co-located and self-documenting.
✏️ Learnings added
Learnt from: EtanHey
Repo: EtanHey/brainlayer PR: 100
File: src/brainlayer/enrichment_controller.py:175-199
Timestamp: 2026-03-22T15:55:19.029Z
Learning: In `src/brainlayer/enrichment_controller.py`, the `parallel` parameter in `enrich_local()` is intentionally kept in the function signature (currently unused, suppressed with `# noqa: ARG001`) for API stability. Parallel local enrichment via a thread pool or process pool is planned for a future iteration. Do not flag this as dead code requiring removal.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T23:35:42.084Z
Learning: Applies to src/brainlayer/enrichment*.py : For enrichment backend configuration: use primary backend MLX (`Qwen2.5-Coder-14B-Instruct-4bit`) on Apple Silicon (port 8080), fallback to Ollama (`glm-4.7-flash`) on port 11434 after 3 consecutive MLX failures, or override with `BRAINLAYER_ENRICH_BACKEND=ollama|mlx|groq` environment variable
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-14T02:20:54.656Z
Learning: Request codex review, cursor review, and bugbot review for BrainLayer PRs
| def test_cli_enrich_mode_realtime_routes_to_controller(monkeypatch): | ||
| monkeypatch.setattr("brainlayer.cli.get_db_path", lambda: "/tmp/test.db") | ||
| monkeypatch.setattr("brainlayer.vector_store.VectorStore", lambda path: MagicMock()) | ||
| called = {} | ||
|
|
||
| def fake_realtime(store, limit=25, since_hours=24, **kwargs): | ||
| called.update({"store": store, "limit": limit, "since_hours": since_hours, **kwargs}) | ||
| return SimpleNamespace(mode="realtime", attempted=1, enriched=1, skipped=0, failed=0, errors=[]) | ||
|
|
||
| monkeypatch.setattr("brainlayer.enrichment_controller.enrich_realtime", fake_realtime) | ||
|
|
||
| result = runner.invoke(app, ["enrich", "--mode", "realtime", "--limit", "9", "--since-hours", "12"]) | ||
|
|
||
| assert result.exit_code == 0 | ||
| assert called["limit"] == 9 | ||
| assert called["since_hours"] == 12 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how CLI imports enrichment_controller functions
rg -n "from.*enrichment_controller import" src/brainlayer/cli/__init__.pyRepository: EtanHey/brainlayer
Length of output: 153
Fix monkeypatch target to patch where the function is imported.
The test patches brainlayer.enrichment_controller.enrich_realtime, but the CLI imports it directly at line 893 of src/brainlayer/cli/__init__.py with from ..enrichment_controller import enrich_realtime. This creates a binding in the cli namespace at import time, so patching the original module location will not affect the CLI's reference. Patch brainlayer.cli.enrich_realtime instead (and apply the same fix to the other two monkeypatch calls for enrich_batch and enrich_local).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_cli_enrich.py` around lines 13 - 28, The test monkeypatches the
original enrichment_controller functions but the CLI imports them into its own
namespace, so update the monkeypatch targets in
test_cli_enrich_mode_realtime_routes_to_controller to patch the CLI bindings
(use "brainlayer.cli.enrich_realtime" instead of
"brainlayer.enrichment_controller.enrich_realtime") and apply the same change
for the other two tests/monkeypatches that replace enrich_batch and enrich_local
so they patch "brainlayer.cli.enrich_batch" and "brainlayer.cli.enrich_local"
respectively.
There was a problem hiding this comment.
Valid — the CLI tests mock at the controller level to avoid hitting real DBs. Will add a missing import assertion test in follow-up.
There was a problem hiding this comment.
@EtanHey, understood — just a quick note to clarify the risk: when brainlayer.cli does from ..enrichment_controller import enrich_realtime, the name enrich_realtime is bound in the brainlayer.cli namespace at import time. Patching brainlayer.enrichment_controller.enrich_realtime replaces the object in the source module but not the CLI's already-resolved reference. This means the test could silently call the real function (and fail or pass for the wrong reason) rather than your fake, regardless of DB access.
The fix remains straightforward — patch brainlayer.cli.enrich_realtime (and similarly brainlayer.cli.enrich_batch / brainlayer.cli.enrich_local).
Would you like me to open a follow-up GitHub issue to track this?
🧠 Learnings used
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-14T02:20:54.656Z
Learning: Request codex review, cursor review, and bugbot review for BrainLayer PRs
| def test_get_enrichment_candidates_honors_explicit_chunk_ids_filter(tmp_path): | ||
| store = VectorStore(tmp_path / "test.db") | ||
| _insert_chunk(store, "c1", "a" * 80) | ||
| _insert_chunk(store, "c2", "b" * 80) | ||
| _insert_chunk(store, "c3", "c" * 80) | ||
|
|
||
| results = store.get_enrichment_candidates(limit=10, chunk_ids=["c1", "c3"]) | ||
|
|
||
| assert {row["id"] for row in results} == {"c1", "c3"} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding test for chunk_ids=[] edge case.
The implementation at line 51-52 in session_repo.py returns [] when chunk_ids is an empty list (defensive early return). A test would document this behavior:
💚 Suggested test
def test_get_enrichment_candidates_returns_empty_for_empty_chunk_ids(tmp_path):
store = VectorStore(tmp_path / "test.db")
_insert_chunk(store, "c1", "a" * 80)
results = store.get_enrichment_candidates(limit=10, chunk_ids=[])
assert results == []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_session_enrichment_candidates.py` around lines 82 - 90, Add a unit
test that verifies VectorStore.get_enrichment_candidates returns an empty list
when called with chunk_ids=[], mirroring the defensive early return in
session_repo.py; specifically add a test (e.g.,
test_get_enrichment_candidates_returns_empty_for_empty_chunk_ids) that creates a
VectorStore, inserts at least one chunk via _insert_chunk, calls
store.get_enrichment_candidates(limit=10, chunk_ids=[]), and asserts the result
equals [] to document and guard this behavior for function
get_enrichment_candidates.
There was a problem hiding this comment.
Good catch — the chunk_ids=[] edge case IS handled (returns empty list immediately on line 4 of get_enrichment_candidates), but a dedicated test would document the contract. Added to follow-up.
There was a problem hiding this comment.
@EtanHey — Thanks for confirming! The defensive early return is a good contract to have explicitly tested. Happy to see it captured as a follow-up. The suggested test snippet in my original comment is ready to drop in whenever you circle back to it. 👍
🧠 Learnings used
Learnt from: CR
Repo: EtanHey/brainlayer PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-14T02:20:54.656Z
Learning: Request codex review, cursor review, and bugbot review for BrainLayer PRs
Fixes from Macroscope + Codex review (round 1): - [P1] Wrap enrich_realtime() in run_in_executor to avoid blocking MCP async event loop (was synchronous, digest path already used executor) - [HIGH] Add GOOGLE_API_KEY placeholder to LaunchAgent plist and install.sh substitution — realtime mode needs Gemini, not Groq - [MEDIUM] Fix dict merge order so custom faceted_enrich_fn can't override status="enriched" via stale key in returned dict Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
|
@codex review |
✅ Actions performedReview triggered.
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: caa8859fc6
ℹ️ 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".
| enrich_result = await loop.run_in_executor( | ||
| None, lambda: enrich_realtime(store=store, limit=limit) | ||
| ) |
There was a problem hiding this comment.
Open a fresh VectorStore inside the executor thread
When brain_digest is called with mode="enrich", this passes the global VectorStore instance into run_in_executor(). That store was created on the event-loop thread, but VectorStore documents that APSW connections are not thread-safe (src/brainlayer/vector_store.py:587), so real MCP calls can fail with cross-thread/busy errors as soon as enrich_realtime() touches store.conn. The fix is to create the store inside the worker thread (or pass only db_path), the same way _store() handles background work.
Useful? React with 👍 / 👎.
| ensure_checkpoint_table(store) | ||
| pending = get_pending_jobs(store) if phase in {"poll", "run"} else [] | ||
| export_files = get_unsubmitted_export_files(db_path=getattr(store, "db_path", None)) if phase in {"submit", "run"} else [] | ||
| attempted = len(pending) + len(export_files) | ||
| return EnrichmentResult(mode="batch", attempted=attempted, enriched=0, skipped=0, failed=0, errors=[]) |
There was a problem hiding this comment.
Implement batch mode instead of returning a placeholder result
This new controller branch never submits, polls, or imports any Gemini batch work: it only counts pending/export files and returns enriched=0. After this commit, the deprecated scripts/cloud_backfill.py tells users to switch to brainlayer enrich --mode batch, so those callers now hit a no-op and backlog chunks never get enriched. I checked scripts/cloud_backfill.py; the real work lives in its export/submit/poll/import helpers, and none of them are invoked here.
Useful? React with 👍 / 👎.
| store.update_enrichment( | ||
| chunk_id=chunk_id, | ||
| summary=summary, | ||
| tags=merged_tags, | ||
| intent=_activity_to_intent(faceted_result.get("activity")), |
There was a problem hiding this comment.
Keep digest-time tags from blocking full enrichment later
For any digest whose faceted tagging succeeds, this calls update_enrichment(), which stamps enriched_at even though only summary, tags, and maybe intent are populated here. get_enrichment_candidates() now selects strictly on enriched_at IS NULL (src/brainlayer/session_repo.py:56), so those digest-created chunks are permanently skipped by the full enrichment pipeline and never receive fields like importance, resolved_query, or primary_symbols. That regresses search/ranking quality for digested content unless partial digest enrichment is tracked separately.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These files are unrelated to the enrichment consolidation PR: - .claude/commands/ symlinks (local Claude Code config) - scripts/.kg_rebuild_progress.json (KG rebuild state) - scripts/backfill_orchestrate.sh (separate utility) - tests/eval_mcp_brainlayer.json (eval harness data) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| if not isinstance(activity, str) or not activity.startswith("act:"): | ||
| return None | ||
| activity = activity.strip().lower() |
There was a problem hiding this comment.
🟡 Medium pipeline/digest.py:182
The activity validation at line 182 uses activity.startswith("act:") before the .lower() normalization on line 184, so uppercase prefixes like "ACT:designing" fail validation and return None even though they should be accepted after normalization.
| if not isinstance(activity, str) or not activity.startswith("act:"): | |
| return None | |
| activity = activity.strip().lower() | |
| activity = payload.get("activity") | |
| if not isinstance(activity, str) or not activity.strip().lower().startswith("act:"): | |
| return None | |
| activity = activity.strip().lower() |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file src/brainlayer/pipeline/digest.py around lines 182-184:
The `activity` validation at line 182 uses `activity.startswith("act:")` before the `.lower()` normalization on line 184, so uppercase prefixes like `"ACT:designing"` fail validation and return `None` even though they should be accepted after normalization.
Evidence trail:
src/brainlayer/pipeline/digest.py lines 181-184 at REVIEWED_COMMIT: Line 182 shows `if not isinstance(activity, str) or not activity.startswith("act:"):` and line 184 shows `activity = activity.strip().lower()`. The validation check happens before the normalization.
| def enrich_batch( | ||
| store, | ||
| phase: str = "run", | ||
| limit: int = 5000, | ||
| max_retries: int = 12, # noqa: ARG001 | ||
| ) -> EnrichmentResult: | ||
| """Process backlog via Gemini Batch API.""" | ||
| ensure_checkpoint_table(store) | ||
| pending = get_pending_jobs(store) if phase in {"poll", "run"} else [] | ||
| export_files = ( | ||
| get_unsubmitted_export_files(db_path=getattr(store, "db_path", None)) if phase in {"submit", "run"} else [] | ||
| ) |
There was a problem hiding this comment.
🟡 Medium brainlayer/enrichment_controller.py:161
enrich_batch accepts a limit parameter but ignores it when calling get_pending_jobs and get_unsubmitted_export_files, so the --limit flag from the CLI has no effect. Consider passing limit to these calls, or remove the parameter if batch processing is intentionally unbounded.
- pending = get_pending_jobs(store) if phase in {"poll", "run"} else []
+ pending = get_pending_jobs(store, limit=limit) if phase in {"poll", "run"} else []
export_files = (
- get_unsubmitted_export_files(db_path=getattr(store, "db_path", None)) if phase in {"submit", "run"} else []
+ get_unsubmitted_export_files(db_path=getattr(store, "db_path", None), limit=limit) if phase in {"submit", "run"} else []
)🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file src/brainlayer/enrichment_controller.py around lines 161-172:
`enrich_batch` accepts a `limit` parameter but ignores it when calling `get_pending_jobs` and `get_unsubmitted_export_files`, so the `--limit` flag from the CLI has no effect. Consider passing `limit` to these calls, or remove the parameter if batch processing is intentionally unbounded.
Evidence trail:
src/brainlayer/enrichment_controller.py lines 161-174 (REVIEWED_COMMIT): `enrich_batch` function shows `limit: int = 5000` parameter in signature at line 163, but `limit` is never used in the function body. Lines 168-170 show `get_pending_jobs(store)` and `get_unsubmitted_export_files(db_path=...)` called without any limit. Compare to `enrich_local` at lines 176-179 which properly passes `limit=limit` to `get_enrichment_candidates`.
…dd -A) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move mode and content validation before _get_vector_store() call. This prevents test_brain_digest_missing_content_with_mode_digest_errors from creating a real DB at DEFAULT_DB_PATH, which was causing test_eval_baselines::test_avi_simon_entity to find the DB and fail instead of skipping (empty DB has no entity data). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
test_cli_enrich_invalid_mode_rejected creates VectorStore(get_db_path()) before hitting BadParameter for invalid mode. On CI this creates an empty DB at the default path, causing test_eval_baselines::test_avi_simon_entity to find the DB and fail instead of skipping. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
README.md: - Test badge: 1030 -> 1137 (1,083 Python + 54 Swift) - Highlight line: 1,002 Python + 28 Swift -> 1,083 Python + 54 Swift - Testing section: 1,002 -> 1,083 - BrainBar Swift tests: 28 -> 54 - brain_digest: document digest and enrich modes (added in PR #100) CLAUDE.md: - Remove orchestrator path reference (private) - Remove orchestrator-specific compact instructions (not relevant to this repo) - BrainBar stubs: 4 -> 3 (brain_digest fixed in PR #100) - MCP tools: 8 -> 9 (was missing brain_tags) - Add brain_tags to tool list - Remove private repo path (golems/scripts/enrichment-lazy.sh) CONTRIBUTING.md: - MCP server comment: 14 tools -> 9 (legacy aliases consolidated) - Test count: 268 -> 1,083 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
enrichment_controller.pywith realtime/batch/local modesbrain_digestMCP tool withmode="enrich"so agents can trigger enrichment directlydom:*+act:*) to the digest pipeline with Gemini + PII sanitizationbrainlayer enrich --mode realtime|batch|local) and LaunchAgent to route through the new controllerenrichment_backfill.pyandcloud_backfill.pywith warnings pointing to new entry pointsSafety rails enforced across all modes
thinkingBudget: 0on ALL Gemini calls (cost incident prevention)build_external_prompt()for PII sanitization before any external API callparse_enrichment()shared schema validationupdate_enrichment()single write pathNew files
src/brainlayer/enrichment_controller.pyenrich_realtime,enrich_batch,enrich_localtests/test_enrichment_controller.pytests/test_session_enrichment_candidates.pyget_enrichment_candidates()teststests/test_mcp_digest_modes.pytests/test_cli_enrich.pytests/test_digest_enrichment.pyModified files
session_repo.py— newget_enrichment_candidates()(single SQL source of truth)mcp/__init__.py— expandedbrain_digestdescription +mode/limitparamsmcp/store_handler.py—mode="enrich"routing to controllercli/__init__.py—brainlayer enrichrewired to--mode realtime|batch|localpipeline/digest.py— faceted Gemini enrichment step indigest_content()pipeline/enrichment.py—build_external_prompt()accepts custom templatesscripts/launchd/com.brainlayer.enrich.plist— new CLI contract,RunAtLoad=trueTest plan
plutil -lint)🤖 Generated with Claude Code
Note
Add unified enrichment controller with realtime, batch, and local modes to
brainlayer enrichrealtime(Gemini API with rate limiting),batch(cloud job inspection via checkpoint), andlocal(local LLM backend). Thebrainlayer enrichCLI now routes to these modes via--mode.digest_contentin digest.py now performs faceted enrichment using Gemini after ingestion, writing tags, intent, and enrichment metadata to the DB and returning them in the result.brain_digestMCP tool gains amodeparameter (digest|enrich) andlimit, allowing enrichment-only runs without providing content.get_enrichment_candidatesadded to session_repo.py with filtering by recency, source, content type, and explicit chunk IDs.DeprecationWarningon import, pointing to the new CLI.--mode realtime, and exportsGOOGLE_API_KEYinstead ofGROQ_API_KEY.Macroscope summarized 6ed7165.
Summary by CodeRabbit
New Features
Deprecations
Chores
Tests