feat: port MCP output formatters to Swift with ANSI colors#132
Conversation
Tags are now intentionally passed through to compact search results for the formatted display. Update the dropped_keys assertion. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Port all 7 Python _format.py formatters to Swift Formatters.swift: - formatSearchResults: box-drawing table with scores, projects, tags - formatStoreResult: one-liner with checkmark - formatEntityCard: rich entity card with profile, relations, memories - formatEntitySimple: lighter entity card - formatStats: knowledge base stats - formatDigestResult: digest/enrich results with action items - formatKGSearch: entity-aware KG search with facts graph ANSI color codes for terminal readability: - Orange (232,121,36) for values (scores, counts) - Blue (88,166,255) for keys/labels (score:, imp:, tags:) - Green (63,185,80) for numbers (chunk counts, result counts) - Gated behind useColor parameter (default true) Wire formatters into MCPRouter so brain_search and brain_store return formatted text instead of raw JSON. 21 new FormattersTests + 13 updated MCPRouterTests = 62/62 green. 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.
📝 WalkthroughWalkthroughAdded a new text-formatting utility (Formatters) that renders MCP response payloads as ANSI/Unicode box-drawn strings and updated router handlers and tests to emit and validate formatted text instead of JSON payloads. Tests for all formatters were added and some router/test assertions adjusted. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Router as MCPRouter
participant Fmt as Formatters
participant Response as ResponsePayload
Client->>Router: POST /brain_search (query, filters)
Router->>Fmt: formatSearchResults(query, results, total, useColor)
Fmt-->>Router: formattedText (ANSI/box drawings)
Router->>Response: build MCP response with formattedText
Response-->>Client: return formatted text payload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
brain-bar/Sources/BrainBar/MCPRouter.swift (1)
13-23: 🧹 Nitpick | 🔵 TrivialRemove unused
StoreResultPayloadstruct.The
StoreResultPayloadstruct is no longer referenced anywhere in the codebase after the refactor to useFormatters.formatStoreResult(). Remove lines 13-23.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brain-bar/Sources/BrainBar/MCPRouter.swift` around lines 13 - 23, Remove the now-unused StoreResultPayload struct declaration: delete the private struct StoreResultPayload { ... } (the type with chunkID, rowID, status and its CodingKeys) since the repo now uses Formatters.formatStoreResult() instead; ensure no remaining references to StoreResultPayload exist and run a build to confirm no usage remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@brain-bar/Sources/BrainBar/Formatters.swift`:
- Around line 61-66: formatNumber currently allocates a new NumberFormatter on
each call; change it to reuse a cached formatter by adding a static/lazy stored
NumberFormatter (e.g. static let cachedFormatter or static var sharedFormatter)
inside the same type and have formatNumber call that instead of creating a new
instance. Ensure the cached formatter is configured once (numberStyle =
.decimal, groupingSeparator = ",") and referenced by formatNumber; if
formatNumber may be called from background threads, either document main-thread
use or use a thread-local or synchronization strategy when accessing the shared
formatter.
---
Outside diff comments:
In `@brain-bar/Sources/BrainBar/MCPRouter.swift`:
- Around line 13-23: Remove the now-unused StoreResultPayload struct
declaration: delete the private struct StoreResultPayload { ... } (the type with
chunkID, rowID, status and its CodingKeys) since the repo now uses
Formatters.formatStoreResult() instead; ensure no remaining references to
StoreResultPayload exist and run a build to confirm no usage remains.
🪄 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: e96420f0-ecba-4883-b4ab-9f1d6e1cca88
📒 Files selected for processing (5)
brain-bar/Sources/BrainBar/Formatters.swiftbrain-bar/Sources/BrainBar/MCPRouter.swiftbrain-bar/Tests/BrainBarTests/FormattersTests.swiftbrain-bar/Tests/BrainBarTests/MCPRouterTests.swifttests/test_search_routing.py
💤 Files with no reviewable changes (1)
- tests/test_search_routing.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.12)
- GitHub Check: test (3.13)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-03-18T00:12:36.931Z
Learnt from: EtanHey
Repo: EtanHey/brainlayer PR: 87
File: brain-bar/Sources/BrainBar/MCPRouter.swift:0-0
Timestamp: 2026-03-18T00:12:36.931Z
Learning: In `brain-bar/Sources/BrainBar/MCPRouter.swift` (Swift, BrainBar MCP daemon), the notification guard `let isNotification = (rawID == nil || rawID is NSNull)` is the single and only point where a no-response decision is made. Any message that passes this guard has a non-nil, non-NSNull id and MUST return a proper JSON-RPC response. Returning `[:]` (empty dict = no response) anywhere after the notification guard is always a bug — it creates a silent client hang. Flag any `return [:]` that appears after the guard in future reviews.
Applied to files:
brain-bar/Sources/BrainBar/MCPRouter.swiftbrain-bar/Tests/BrainBarTests/MCPRouterTests.swift
📚 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.swiftbrain-bar/Sources/BrainBar/Formatters.swift
📚 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:
brain-bar/Tests/BrainBarTests/MCPRouterTests.swift
🪛 SwiftLint (0.63.2)
brain-bar/Tests/BrainBarTests/FormattersTests.swift
[Warning] 9-9: Classes should have an explicit deinit method
(required_deinit)
🔇 Additional comments (7)
brain-bar/Sources/BrainBar/Formatters.swift (1)
1-396: LGTM!The formatters are well-structured and match the Python reference implementation. The code handles edge cases (empty results, nil values, multiple input formats for tags) correctly and uses appropriate truncation limits. The ANSI color gating via
useColorparameter is clean.brain-bar/Tests/BrainBarTests/FormattersTests.swift (1)
1-297: LGTM!Comprehensive test coverage for all 7 formatters with good edge case testing (empty results, minimal entities, color toggling). The tests verify both content correctness and structural elements (box-drawing characters, ANSI escape sequences).
brain-bar/Sources/BrainBar/MCPRouter.swift (2)
233-233: LGTM!The change from JSON serialization to
Formatters.formatSearchResults()aligns with the Python MCP behavior (seesrc/brainlayer/mcp/_format.py). The formatter receives the necessary parameters (query, results, total count).
246-246: LGTM!The change to use
Formatters.formatStoreResult()matches the Python implementation's approach. Note that thesupersededandqueuedparameters are not passed here—if those scenarios apply, you may need to wire them through later.brain-bar/Tests/BrainBarTests/MCPRouterTests.swift (3)
228-236: LGTM!The updated assertions correctly verify the formatted text output by checking for the box-drawing header character and the presence/absence of expected chunk IDs. This aligns with the router's switch from JSON to formatted text.
260-267: LGTM!Consistent pattern with the project filter test—verifies formatted output structure and chunk ID filtering by importance.
301-309: LGTM!Good awareness of the substring issue (line 308 comment). The test correctly verifies that unread filtering works by checking for
"unread-1"in the output. The assertion on line 309 checking for"result"provides a lightweight sanity check that formatted output was produced.
|
@coderabbitai review |
|
@codex review |
✅ Actions performedReview triggered.
|
|
Excellent! All three fixes are correctly applied. Let me create a summary of what was done: SummaryI've successfully addressed all code review issues identified by @coderabbitai and @macroscopeapp in PR #132: Fixes Applied (commit 94c6c55)
All changes have been committed and pushed to the   |
- Use 'content' field instead of non-existent 'snippet' field in formatSearchResults - Cache NumberFormatter to avoid repeated allocations - Remove unused StoreResultPayload struct Co-authored-by: Etan Heyman <EtanHey@users.noreply.github.com>
![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: ddeb942574
ℹ️ 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".
| } | ||
| let stored = try db.store(content: content, tags: tags, importance: importance, source: "mcp") | ||
| return jsonEncode(StoreResultPayload(chunkID: stored.chunkID, rowID: stored.rowID, status: "stored")) | ||
| return Formatters.formatStoreResult(chunkId: stored.chunkID) |
There was a problem hiding this comment.
Return a structured payload here instead of only formatted text: BrainBarServer.publishStoredChunk still calls extractStoredChunk to JSON-decode result.content[0].text into StoreResultPayload, and that decode now fails for every successful brain_store response ("✔ Stored → ..."). In the tagged-subscription path this causes the guard in publishStoredChunk to exit early, so resource/channel notifications and unread/ack sequencing are skipped whenever clients store memories through MCP.
Useful? React with 👍 / 👎.
| lines.append("\u{2502}") | ||
|
|
||
| for (i, r) in results.enumerated() { | ||
| let score = r["score"] as? Double ?? 0 |
There was a problem hiding this comment.
Avoid defaulting to 0 for r["score"] in search output unless the DB actually provides scores: BrainDatabase.search populates result dictionaries without a score field, so this formatter renders score:0.00 for every hit regardless of ranking. That is a retrieval-quality regression in user-visible output because it presents fabricated relevance values as if they were real.
Useful? React with 👍 / 👎.
| private static func pad(_ text: String?, width: Int, align: Alignment = .left) -> String { | ||
| let t = text ?? "" | ||
| let s = t.count > width ? String(t.prefix(width - 1)) + "\u{2026}" : t | ||
| switch align { | ||
| case .left: return s.padding(toLength: width, withPad: " ", startingAt: 0) |
There was a problem hiding this comment.
🟢 Low BrainBar/Formatters.swift:46
The pad function uses inconsistent length semantics across alignment modes. For .left, it calls padding(toLength:withPad:startingAt:) which counts UTF-16 code units (inherited from NSString), but the t.count > width check and the .right/.center cases use s.count which counts Swift Character grapheme clusters. For strings containing emoji or combining characters, .left alignment produces a different visual width than .right or .center, breaking table alignment in terminal output. Consider using a consistent length metric — either convert all cases to use NSString length (e.g., (s as NSString).length) or use Swift Character count consistently by reimplementing left-padding without padding(toLength:).
- case .left: return s.padding(toLength: width, withPad: " ", startingAt: 0)🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/Formatters.swift around lines 46-50:
The `pad` function uses inconsistent length semantics across alignment modes. For `.left`, it calls `padding(toLength:withPad:startingAt:)` which counts UTF-16 code units (inherited from `NSString`), but the `t.count > width` check and the `.right`/`.center` cases use `s.count` which counts Swift `Character` grapheme clusters. For strings containing emoji or combining characters, `.left` alignment produces a different visual width than `.right` or `.center`, breaking table alignment in terminal output. Consider using a consistent length metric — either convert all cases to use `NSString` length (e.g., `(s as NSString).length`) or use Swift `Character` count consistently by reimplementing left-padding without `padding(toLength:)`.
Evidence trail:
1. brain-bar/Sources/BrainBar/Formatters.swift lines 46-56 (REVIEWED_COMMIT) - shows the `pad` function with `.left` using `padding(toLength:)` and `.right`/`.center` using `s.count`
2. Apple Developer Documentation for NSString: https://developer.apple.com/documentation/foundation/nsstring - states "All lengths, character indexes, and ranges are expressed in terms of 16-bit platform-endian values, with index values starting at 0" and "var length : Int - The number of UTF-16 code units in the receiver"
3. `padding(toLength:withPad:startingAt:)` is documented as an NSString method: https://developer.apple.com/documentation/foundation/nsstring/padding(tolength:withpad:startingat:)
2 participants
Summary
_format.pyformatters to SwiftFormatters.swift(396 LOC)useColorparam)brain_searchandbrain_storein MCPRouter to return formatted text instead of raw JSONFiles Changed
Formatters.swift(new, 396 LOC)MCPRouter.swift(+3/-5)handleBrainSearchandhandleBrainStoreFormattersTests.swift(new, 297 LOC)MCPRouterTests.swift(+19/-16)Test plan
🤖 Generated with Claude Code
Note
Port MCP output formatters to Swift with ANSI colors and box-drawing for brain tool responses
Formattersenum in Formatters.swift with static methods to render brain tool outputs (brain_search,brain_store, entity cards, stats, digest, KG search) as human-readable terminal strings using Unicode box-drawing and optional ANSI colors.MCPRoutersobrain_searchreturns formatted text instead of a raw JSON array, andbrain_storereturns a formatted status line instead of a JSON object.tools/callresponses forbrain_searchandbrain_storeare no longer JSON-parseable — callers expecting structured JSON will break.Macroscope summarized 94c6c55.
Summary by CodeRabbit
New Features
Tests