feat: add Core-4 MCP tool palette#594
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_eb3c6713-dac7-4500-baff-adddc9084620) |
|
@coderabbitai review |
|
@BugBot review |
|
@codex review |
|
@cursor review |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ffc80501-161c-4a97-9911-8d478e9c1800) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_bc4a011b-cd00-41dd-9a9c-67af7ac149b0) |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughMCP tool exposure now supports core and full profiles, session-level palette expansion, exposed-tool validation, and list-change signaling in BrainBar and Python. Contracts, design plans, measurement tooling, dependencies, and tests were updated for the new behavior. ChangesMCP palette behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPServer
participant ToolPalette
Client->>MCPServer: tools/list
MCPServer->>ToolPalette: expose full tool definitions
ToolPalette-->>MCPServer: exposed core or full tools
MCPServer-->>Client: tools list
Client->>MCPServer: tools/call expand_palette
MCPServer->>ToolPalette: expand palette
ToolPalette-->>MCPServer: expansion receipt
MCPServer-->>Client: structured receipt and updated list state
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e4ace1f6f
ℹ️ 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".
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c1991af7-8ec6-4e23-8762-4dcee7492d55) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@brain-bar/Sources/BrainBar/MCPRouter.swift`:
- Around line 201-223: Update compactCoreToolDefinition for brain_recall and
brain_expand to preserve the anthropic/maxResultSizeChars annotation with value
250,000 while retaining the existing description and inputSchema compaction.
Confirm the resulting Core4 definitions still satisfy the enforced 1,500-byte
budget; if they do not, use the smallest targeted representation that preserves
this hint for these two tools without restoring unrelated annotations.
- Around line 399-413: Update toolCallResult to protect reserved response keys
when merging output.metadata. Define or reuse a reserved-key set covering
content, isError, jsonrpc, id, and result, then filter or assert metadata
entries before assigning them so metadata cannot overwrite the response body or
JSON-RPC fields.
🪄 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: 5c1b0537-62bb-4535-8b60-a381165b9e6e
📒 Files selected for processing (22)
brain-bar/Sources/BrainBar/MCPRouter.swiftbrain-bar/Tests/BrainBarTests/MCPRouterTests.swiftbrain-bar/Tests/BrainBarTests/SocketIntegrationTests.swiftcontracts/engine-ui-contract.mddocs/plans/2026-07-13-brainlayer-core4-palette-design.mddocs/plans/2026-07-13-brainlayer-core4-palette.mdpyproject.tomlscripts/measure_mcp_palette.pysrc/brainlayer/mcp/__init__.pysrc/brainlayer/mcp/palette.pytests/test_3tool_aliases.pytests/test_audit_search_quality.pytests/test_entity_type_sync.pytests/test_mcp_digest_modes.pytests/test_mcp_input_schema_limits.pytests/test_mcp_palette.pytests/test_measure_mcp_palette.pytests/test_phase3_digest.pytests/test_search_filter_params.pytests/test_think_recall_integration.pytests/test_tool_annotations.pytests/test_tool_description_quality.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: swift (macos-15)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- 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:
tests/test_think_recall_integration.pytests/test_search_filter_params.pytests/test_tool_annotations.pytests/test_mcp_palette.pytests/test_phase3_digest.pytests/test_audit_search_quality.pytests/test_entity_type_sync.pysrc/brainlayer/mcp/palette.pytests/test_mcp_digest_modes.pytests/test_mcp_input_schema_limits.pyscripts/measure_mcp_palette.pytests/test_tool_description_quality.pytests/test_3tool_aliases.pytests/test_measure_mcp_palette.pysrc/brainlayer/mcp/__init__.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Do not run
tests/test_vector_store.pyortests/test_engine.pyduring worker pre-push checks; run them only as deliberate production-database checks.
Files:
tests/test_think_recall_integration.pytests/test_search_filter_params.pytests/test_tool_annotations.pytests/test_mcp_palette.pytests/test_phase3_digest.pytests/test_audit_search_quality.pytests/test_entity_type_sync.pytests/test_mcp_digest_modes.pytests/test_mcp_input_schema_limits.pytests/test_tool_description_quality.pytests/test_3tool_aliases.pytests/test_measure_mcp_palette.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Usepaths.py:get_db_path()for database path resolution in all scripts and CLI code.
Retry onSQLITE_BUSY; each worker must use its own database connection.
Files:
src/brainlayer/mcp/palette.pysrc/brainlayer/mcp/__init__.py
src/brainlayer/mcp/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/mcp/**/*.py: Use the fixed-size read-only WALVectorStorepool; respectBRAINLAYER_READ_POOL_SIZE,BRAINLAYER_READ_BUSY_TIMEOUT_MS, and the approximately 768 MB pool/cache startup limit.
Add new MCP tool implementations in themcp/directory and expose them through thebrainlayer-mcpentrypoint.
Files:
src/brainlayer/mcp/palette.pysrc/brainlayer/mcp/__init__.py
src/brainlayer/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/brainlayer/**/*.py: Preserveai_code,stack_trace, anduser_messageverbatim; skip noise, summarize build logs, and retain only structure for directory listings.
Use AST-aware tree-sitter chunking, never split stack traces, and mask large tool output.
Use Groq as the primary enrichment backend, Gemini as fallback, and Ollama as the offline last resort; honorBRAINLAYER_ENRICH_BACKENDandBRAINLAYER_ENRICH_RATE.
Default searches must exclude lifecycle-managed chunks;include_archived=Truemay show history.
brain_supersedemust apply a safety gate to personal data, andbrain_archivemust perform a timestamped soft delete.
brain_storemust support an atomicsupersedesstore-and-replace operation.
Production backup logs must identify real runs withbackup_log_provenance=real; tests must useBRAINLAYER_BACKUP_LOG_PATHand provenancepytestso they cannot modify the production heartbeat log.
Before bulk database operations, stop enrichment workers, checkpoint WAL before and after withPRAGMA wal_checkpoint(FULL), drop and recreate FTS triggers around large deletes, batch deletes in 5–10K chunks, and checkpoint every three batches.
Files:
src/brainlayer/mcp/palette.pysrc/brainlayer/mcp/__init__.py
🧠 Learnings (2)
📚 Learning: 2026-03-18T00:12:08.774Z
Learnt from: EtanHey
Repo: EtanHey/brainlayer PR: 87
File: brain-bar/Sources/BrainBar/BrainBarServer.swift:118-129
Timestamp: 2026-03-18T00:12:08.774Z
Learning: In Swift files under brain-bar/Sources/BrainBar, enforce that when a critical dependency like the database is nil due to startup ordering (socket before DB), any tool handler that accesses the database must throw an explicit error (e.g., ToolError.noDatabase) instead of returning a default/empty value. Do not allow silent defaults (e.g., guard let db else { return ... }). Flag patterns that silently return defaults when db is nil, as this masks startup timing issues. This guidance applies broadly to similar Swift files in the BrainBar module, not just this one location.
Applied to files:
brain-bar/Sources/BrainBar/MCPRouter.swift
📚 Learning: 2026-03-29T18:45:40.988Z
Learnt from: EtanHey
Repo: EtanHey/brainlayer PR: 133
File: brain-bar/Sources/BrainBar/BrainDatabase.swift:0-0
Timestamp: 2026-03-29T18:45:40.988Z
Learning: In the BrainBar module’s Swift database layer (notably BrainDatabase.swift), ensure that the `search()` function’s `unreadOnly=true` path orders results by the delivery frontier cursor so the watermark `maxRowID` stays contiguous. Specifically, when `unreadOnly` is enabled, the query must include `ORDER BY c.rowid ASC` (e.g., via `let orderByClause = unreadOnly ? "c.rowid ASC" : "f.rank"`). Do not replace the unread-only ordering with relevance-based sorting (e.g., `f.rank`) unconditionally or for the unread-only path, as it can introduce gaps in the watermark and incorrectly mark unseen rows as delivered. Flag any future change to the `ORDER BY` clause in this function that makes relevance sorting apply to the unread-only case.
Applied to files:
brain-bar/Sources/BrainBar/MCPRouter.swift
🪛 ast-grep (0.44.1)
scripts/measure_mcp_palette.py
[info] 36-36: use jsonify instead of json.dumps for JSON output
Context: json.dumps(tools, ensure_ascii=False, separators=(",", ":"), sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 66-66: use jsonify instead of json.dumps for JSON output
Context: json.dumps(measurement, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/test_measure_mcp_palette.py
[info] 16-16: use jsonify instead of json.dumps for JSON output
Context: json.dumps(tools, ensure_ascii=False, separators=(",", ":"), sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 35-35: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"tools": [{"name": "brain_search"}]})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
src/brainlayer/mcp/__init__.py
[info] 1337-1337: use jsonify instead of json.dumps for JSON output
Context: json.dumps(receipt, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 LanguageTool
docs/plans/2026-07-13-brainlayer-core4-palette-design.md
[style] ~90-~90: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... database, schema, or writer changes. - No deletion or renaming of compatibility t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~91-~91: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...n or renaming of compatibility tools. - No attempt to make profile changes affect ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.23.0)
docs/plans/2026-07-13-brainlayer-core4-palette.md
[warning] 13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (29)
src/brainlayer/mcp/__init__.py (4)
4-4: LGTM!Also applies to: 44-44, 401-402
1333-1340: 🎯 Functional CorrectnessVerify that expansion emits
tools/list_changed.This path changes
list_tools()results but only returns a receipt. If the MCP server does not synthesize the notification automatically, connected clients will retain the core-only tool cache after expansion. Confirm the SDK behavior and add a socket-level test; emit the notification explicitly if required.
1255-1262: 🎯 Functional CorrectnessGlobal palette state is fine here.
serve()runs one stdio MCP server process, andToolPaletteis the intended session state forlist_tools()/expand_palette, so there’s no cross-client palette leak in this setup.> Likely an incorrect or invalid review comment.
1342-1343: 🎯 Functional CorrectnessLegacy
brainlayer_*aliases are intentional compatibility shims. The core-profile gate applies to canonical tools; the alias dispatch is expected to remain callable for backward compatibility.> Likely an incorrect or invalid review comment.pyproject.toml (1)
85-85: LGTM!scripts/measure_mcp_palette.py (1)
1-75: LGTM!contracts/engine-ui-contract.md (1)
20-44: LGTM!docs/plans/2026-07-13-brainlayer-core4-palette-design.md (1)
1-92: LGTM!docs/plans/2026-07-13-brainlayer-core4-palette.md (1)
1-304: LGTM!src/brainlayer/mcp/palette.py (1)
26-71: 🩺 Stability & AvailabilityConfirm
ToolPaletteis never mutated off the event loop.
_expandedis set onselfwith no lock (Line 66). This is safe only ifexpand()/is_exposed()/expose()always run on the single asyncio event loop thread with no interleavingawaitinside them (true here — none of these methods await). If the MCP server ever dispatchescall_toolthrough a thread pool or multiple event-loop workers share one_tool_paletteinstance, this becomes an unguarded read-modify-write on_expanded.As per coding guidelines: "Flag risky DB or concurrency changes explicitly and do not hand-wave lock behavior." Please confirm (or add a comment noting) that
_tool_paletteis a per-process singleton consumed only on the asyncio event loop thread.Source: Coding guidelines
brain-bar/Sources/BrainBar/MCPRouter.swift (1)
13-38: LGTM!Also applies to: 123-125, 329-329, 349-349, 368-384
brain-bar/Tests/BrainBarTests/MCPRouterTests.swift (4)
61-81: LGTM!Also applies to: 111-138, 140-161
165-170: LGTM!Also applies to: 191-204, 213-230
232-2597: LGTM!
206-211: 🎯 Functional CorrectnessDrop the deferred-tool mismatch concern BrainBar returns a JSON-RPC
-32601for an unexposed tool, while Python returns aCallToolResultwithisError=Trueand a “not exposed” message. The shared palette contract only covers which tools are exposed and howexpand_palettebehaves.> Likely an incorrect or invalid review comment.tests/test_mcp_palette.py (1)
14-36: LGTM!tests/test_measure_mcp_palette.py (1)
1-43: LGTM!tests/test_think_recall_integration.py (1)
245-254: LGTM!tests/test_tool_annotations.py (1)
69-72: LGTM!tests/test_tool_description_quality.py (1)
1-15: LGTM!brain-bar/Tests/BrainBarTests/SocketIntegrationTests.swift (1)
15-38: 🩺 Stability & AvailabilityNo issue:
BRAINLAYER_MCP_PROFILEis restored per test, and SwiftPM/XCTest parallelism runs tests in separate processes, so these mutations don’t create a cross-test race.> Likely an incorrect or invalid review comment.tests/test_3tool_aliases.py (2)
17-19: LGTM!
22-29: 🎯 Functional Correctness
brain_expandstays a deprecation stub.CORE_TOOL_NAMESstill lists it for palette exposure, butcall_tool()returns the deprecated error path, sotest_brain_expand_returns_deprecation_errormatches the runtime behavior.> Likely an incorrect or invalid review comment.tests/test_audit_search_quality.py (1)
125-127: LGTM!tests/test_entity_type_sync.py (1)
31-34: LGTM!tests/test_mcp_input_schema_limits.py (1)
8-14: LGTM!Also applies to: 66-67
tests/test_phase3_digest.py (1)
289-350: LGTM!tests/test_search_filter_params.py (1)
806-808: LGTM!tests/test_mcp_digest_modes.py (1)
2-2: 🎯 Functional CorrectnessNo remaining
asynciousage — this file only uses@pytest.mark.asyncio, so removing the import is safe.> Likely an incorrect or invalid review comment.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1709e326-2648-4fa4-a415-127ea354af22) |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Independent standing-reviewer verdict (s:399) — SHIP ✅ (with one required fast-follow)Reviewed from an isolated worktree at head
CI green (Python 3.11/3.12/3.13, Swift 750, lint, CodeRabbit, Macroscope). Required fast-follow (the one gap): brief item 5's byte-budget REGRESSION GUARD is not wired —
|
1 participant
Summary
brain_search,brain_store,brain_recall, andbrain_expandthe default BrainLayer MCP palette in both BrainBar/Swift and Python.BRAINLAYER_MCP_PROFILE=full(oroperator) and through the idempotentexpand_palettecontrol tool.tools.listChanged, reject deferred canonical tools before expansion, and retain canonical full-profile schemas/annotations unchanged.o200k_basetoken measurement tooling, budget tests, profile/expansion round-trip tests, and contract documentation.Context-budget receipt
expand_paletteThe new default is 2,560 bytes / 659 tokens smaller than the deployed baseline: 63.4% byte reduction and 64.2% token reduction. The verbose repository full-profile figure is reported separately because it is not the same artifact as the deployed audit capture.
The current canonical repository Core-4 schemas were 3,722 bytes before compact projection, so the default projection preserves names and validation-relevant input-schema structure while omitting nested descriptions and annotations. Full/operator output remains canonical.
Verification
All test commands were run with
ulimit -n 4096for the decisive receipts.git diff --check: clean.Unrelated baseline / tooling notes
TestStoreAutoEnrichis a pre-existing untouched-main baseline failure per standing direction. It was not investigated or changed. It did not reproduce in the raised-fd local or pre-push runs.test_brainbar_vacuum_request_accepts_valid_target_after_lost_responsebecause that test monkeypatches the shared stdlibtime.sleepwhile a background 0.25-second loop is alive. The test and implementation are unchanged fromorigin/main; it passed alone and in the raised-fd pre-push gate. No unrelated fix is included.Too many open files; the complete gate was rerun fromulimit -n 4096and passed.Risk / rollout
fullandoperatorare immediate compatibility escape hatches;expand_paletteupgrades the active palette without restart and is idempotent.Note
Medium Risk
Changes default tool visibility for all MCP clients; full/operator env and expand_palette mitigate breakage, but clients assuming the full list without profile or expansion will see fewer tools and failed calls until upgraded.
Overview
Default MCP sessions now advertise a compact Core-4 palette (
brain_search,brain_store,brain_recall,brain_expand) plusexpand_palette, instead of the full BrainBar/Python tool inventories at connect time.BRAINLAYER_MCP_PROFILEselects behavior: missing/blank/core(and unknown values fail closed to core),full/operatorexpose the complete canonical set with no control tool.BrainBar adds per-socket
PaletteSessionstate, routestools/listandtools/callthrough session-aware exposure checks, and setstools.listChanged: true. Server-handled tools (brain_subscribe, etc.) follow the same defer-until-expanded rules. Python addsToolPalettewith matching profile resolution, exposure guards, and idempotentexpand_palettereceipts.expand_palettepromotes only that session to full tools; repeat calls are no-ops. Deferred tools error until expansion. Canonical handler definitions are unchanged; core listings use compact schema projections to meet the boot budget. Addsmeasure_mcp_palette.py, contract/docs updates, and broad test coverage (profile, expansion, per-client isolation, byte budget).Reviewed by Cursor Bugbot for commit 0a86e9b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Core-4 MCP tool palette with per-session expansion across Swift and Python transports
ToolPaletteabstraction (palette.py) andMCPRouter.PaletteSession(MCPRouter.swift) that restrict the default tool set to four core tools plus anexpand_palettecontrol tool whenBRAINLAYER_MCP_PROFILEis unset,core, or unrecognized.expand_paletteonce per session promotes the client to the full tool inventory; subsequent calls returnalready_expandedwithout changing state. BrainBar advertisestools.listChanged=trueso clients can refresh viatools/list.brain_subscribe,brain_unsubscribe,brain_ackin BrainBar; all non-core tools in Python) now return JSON-RPC-32601untilexpand_paletteis called in core profile.Macroscope summarized 0a86e9b.
Summary by CodeRabbit
New Features
Documentation
Tests