feat: BrainBar Knowledge Graph viewer#159
Merged
Merged
Conversation
Canvas-based graph visualization of kg_entities and kg_relations with interactive node selection, entity sidebar with linked chunks, and force-directed physics simulation. TDD: 25 new tests (all green). New files: - KGNode.swift, KGEdge.swift — graph models with importance-scaled radius and type-based colors - KGViewModel.swift — force-directed layout (repulsion + spring + centering), hit-testing, entity selection - KGCanvasView.swift — SwiftUI Canvas renderer with pan/zoom/tap gestures - KGNodeView.swift, KGEdgeView.swift — Canvas draw helpers for nodes and edges - KGSidebarView.swift — entity detail panel showing relations and linked chunks - KnowledgeGraphTests.swift — 25 TDD tests covering DB queries, models, and ViewModel Extended BrainDatabase.swift: - kg_entity_chunks table (entity-chunk junction with relevance) - fetchKGEntities(), fetchKGRelations(), linkEntityChunk(), fetchEntityChunks() - recordInjectionEvent(), listInjectionEvents() (fixes pre-existing compilation gap) Removed BrainBarSupport.swift (duplicate declarations of BrainBarURLAction, HotkeyRouteStatus). 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.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis pull request adds comprehensive Knowledge Graph functionality to BrainBar, including a new interactive graph visualization UI with canvas rendering, physics-based node layout simulation, and sidebar entity details. It extends the database layer with KG-specific tables and data access methods. Legacy URL-routing code is removed. Changes
Sequence DiagramsequenceDiagram
participant User
participant KGCanvasView
participant KGViewModel
participant BrainDatabase
participant Physics Simulation
User->>KGCanvasView: view appears
KGCanvasView->>KGViewModel: loadGraph()
KGViewModel->>BrainDatabase: fetchKGEntities()
BrainDatabase-->>KGViewModel: [KGEntityRow...]
KGViewModel->>BrainDatabase: fetchKGRelations()
BrainDatabase-->>KGViewModel: [KGRelationRow...]
KGViewModel->>KGViewModel: build KGNode & KGEdge objects
KGViewModel-->>KGCanvasView: nodes, edges published
loop ~30fps while on-screen
KGCanvasView->>KGViewModel: tick()
KGViewModel->>Physics Simulation: compute forces (repulsion, spring, center)
Physics Simulation->>KGViewModel: update node velocities & positions
KGViewModel-->>KGCanvasView: nodes updated
KGCanvasView->>KGCanvasView: redraw canvas
end
User->>KGCanvasView: tap node
KGCanvasView->>KGViewModel: nodeAt(point:)
KGViewModel-->>KGCanvasView: KGNode
KGCanvasView->>KGViewModel: selectNode(id:)
KGViewModel->>BrainDatabase: lookupEntity(id:)
BrainDatabase-->>KGViewModel: EntityCard
KGViewModel->>BrainDatabase: fetchEntityChunks(entityId:)
BrainDatabase-->>KGViewModel: [KGChunkRow...]
KGViewModel-->>KGCanvasView: selectedEntity, selectedEntityChunks published
KGCanvasView->>KGCanvasView: redraw with highlighting + update sidebar
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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}
Comment on lines
+13
to
+15
| var radius: CGFloat { | ||
| CGFloat(8 + (importance / 10.0) * 20) | ||
| } |
There was a problem hiding this comment.
🟡 Medium KnowledgeGraph/KGNode.swift:13
The radius computed property does not clamp importance, so values outside 0–10 produce incorrect radii. For example, importance = -5 yields radius = -2 (breaking hit testing and rendering), and importance = 100 yields radius = 208 (far exceeding the documented max of 28). Consider clamping importance to [0, 10] before scaling.
- CGFloat(8 + (importance / 10.0) * 20)
+ CGFloat(8 + (min(max(importance, 0), 10) / 10.0) * 20)🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/KnowledgeGraph/KGNode.swift around lines 13-15:
The `radius` computed property does not clamp `importance`, so values outside 0–10 produce incorrect radii. For example, `importance = -5` yields `radius = -2` (breaking hit testing and rendering), and `importance = 100` yields `radius = 208` (far exceeding the documented max of 28). Consider clamping `importance` to [0, 10] before scaling.
Evidence trail:
brain-bar/Sources/BrainBar/KnowledgeGraph/KGNode.swift lines 12-14 (REVIEWED_COMMIT): Shows radius = 8 + (importance / 10.0) * 20 with documented min 8, max 28. Lines 29-30 show the initializer accepts importance: Double with no validation. Mathematical verification confirms the claims: importance=-5 gives radius=-2, importance=100 gives radius=208.
Comment on lines
+3
to
+8
| struct KGEdge: Identifiable, Equatable { | ||
| let sourceId: String | ||
| let targetId: String | ||
| let relationType: String | ||
|
|
||
| var id: String { "\(sourceId)-\(relationType)-\(targetId)" } |
There was a problem hiding this comment.
🟢 Low KnowledgeGraph/KGEdge.swift:3
The computed id joins sourceId, relationType, and targetId with hyphens, producing collisions when any field contains a hyphen. For example, sourceId: "a-b", relationType: "c", targetId: "d" and sourceId: "a", relationType: "b-c", targetId: "d" both generate id = "a-b-c-d". In ForEach or other Identifiable contexts, this causes one edge to be dropped or incorrectly merged. Consider using a delimiter that cannot appear in the data, or encoding/escaping the fields.
- var id: String { "\(sourceId)-\(relationType)-\(targetId)" }
+ var id: String { "\(sourceId)\u{001F}\(relationType)\u{001F}\(targetId)" }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/KnowledgeGraph/KGEdge.swift around lines 3-8:
The computed `id` joins `sourceId`, `relationType`, and `targetId` with hyphens, producing collisions when any field contains a hyphen. For example, `sourceId: "a-b", relationType: "c", targetId: "d"` and `sourceId: "a", relationType: "b-c", targetId: "d"` both generate `id = "a-b-c-d"`. In `ForEach` or other `Identifiable` contexts, this causes one edge to be dropped or incorrectly merged. Consider using a delimiter that cannot appear in the data, or encoding/escaping the fields.
Evidence trail:
brain-bar/Sources/BrainBar/KnowledgeGraph/KGEdge.swift lines 3-8 at REVIEWED_COMMIT: `struct KGEdge: Identifiable, Equatable` with `var id: String { "\(sourceId)-\(relationType)-\(targetId)" }`. The hyphen delimiter creates collision potential when any of the three fields contain hyphens.
| bindText(query, to: stmt, index: 3) | ||
| bindText(chunkJSON, to: stmt, index: 4) | ||
| sqlite3_bind_int(stmt, 5, Int32(tokenCount)) | ||
| sqlite3_step(stmt) |
There was a problem hiding this comment.
🟢 Low BrainBar/BrainDatabase.swift:1652
In recordInjectionEvent(), the return value of sqlite3_step(stmt) on line 1652 is ignored. When the INSERT fails (e.g., SQLITE_BUSY or disk full), the function still calls sqlite3_last_insert_rowid(db) and returns an InjectionEvent with an invalid id—either 0 or a stale rowid from a previous insert. Callers receive no indication that recording failed, and the id does not correspond to any actual database row.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/BrainDatabase.swift around line 1652:
In `recordInjectionEvent()`, the return value of `sqlite3_step(stmt)` on line 1652 is ignored. When the INSERT fails (e.g., `SQLITE_BUSY` or disk full), the function still calls `sqlite3_last_insert_rowid(db)` and returns an `InjectionEvent` with an invalid `id`—either 0 or a stale rowid from a previous insert. Callers receive no indication that recording failed, and the `id` does not correspond to any actual database row.
Evidence trail:
brain-bar/Sources/BrainBar/BrainDatabase.swift lines 1640-1655 at REVIEWED_COMMIT:
- Line 1652: `sqlite3_step(stmt)` - return value is discarded
- Line 1653: `let rowID = sqlite3_last_insert_rowid(db)` - called unconditionally
- Line 1654: `return InjectionEvent(id: rowID, ...)` - returns potentially invalid id
SQLite documentation confirms `sqlite3_last_insert_rowid()` behavior: returns 0 or stale rowid when INSERT fails.
Comment on lines
+99
to
+104
| private func canvasPoint(from screenPoint: CGPoint, in size: CGSize) -> CGPoint { | ||
| CGPoint( | ||
| x: (screenPoint.x - offset.width) / scale, | ||
| y: (screenPoint.y - offset.height) / scale | ||
| ) | ||
| } |
There was a problem hiding this comment.
🟠 High KnowledgeGraph/KGCanvasView.swift:99
canvasPoint(from:in:) ignores the center-based transform applied in the Canvas block. The canvas translates to center, scales, then translates back, but the inverse function only undoes offset and scale—it never accounts for the centering translations. This causes tapGesture to compute wrong coordinates, so node hit-testing selects the wrong node (or misses entirely) when the view is zoomed or panned. The function needs to subtract size.width/2 before scaling and add it back after, matching the forward transform.
- private func canvasPoint(from screenPoint: CGPoint, in size: CGSize) -> CGPoint {
- CGPoint(
- x: (screenPoint.x - offset.width) / scale,
- y: (screenPoint.y - offset.height) / scale
- )
- }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/KnowledgeGraph/KGCanvasView.swift around lines 99-104:
`canvasPoint(from:in:)` ignores the center-based transform applied in the Canvas block. The canvas translates to center, scales, then translates back, but the inverse function only undoes `offset` and `scale`—it never accounts for the centering translations. This causes `tapGesture` to compute wrong coordinates, so node hit-testing selects the wrong node (or misses entirely) when the view is zoomed or panned. The function needs to subtract `size.width/2` before scaling and add it back after, matching the forward transform.
Evidence trail:
KGCanvasView.swift lines 28-31 (forward transform with centering): `ctx.translateBy(x: offset.width + size.width / 2, ...)`, `ctx.scaleBy(x: scale, y: scale)`, `ctx.translateBy(x: -size.width / 2, ...)`
KGCanvasView.swift lines 99-103 (inverse function): `canvasPoint(from:in:)` only computes `(screenPoint.x - offset.width) / scale` - missing the center-based translations
KGCanvasView.swift line 72: `tapGesture` calls `canvasPoint(from: value.location, in: .zero)` passing `.zero` for size
| } | ||
| } | ||
|
|
||
| private var dragGesture: some Gesture { |
There was a problem hiding this comment.
🟠 High KnowledgeGraph/KGCanvasView.swift:80
The dragGesture overwrites offset with only the current gesture's translation, discarding any previously accumulated offset. After releasing a drag and starting a new one, the canvas jumps back toward the origin because the previous pan position is lost. Consider tracking a base offset and adding the translation to it during drag.
- private var dragGesture: some Gesture {
- DragGesture()
- .onChanged { value in
- offset = CGSize(
- width: value.translation.width,
- height: value.translation.height
- )
- }
- }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/KnowledgeGraph/KGCanvasView.swift around line 80:
The `dragGesture` overwrites `offset` with only the current gesture's translation, discarding any previously accumulated offset. After releasing a drag and starting a new one, the canvas jumps back toward the origin because the previous pan position is lost. Consider tracking a base offset and adding the translation to it during drag.
Evidence trail:
brain-bar/Sources/BrainBar/KnowledgeGraph/KGCanvasView.swift lines 6 (offset declaration), 80-87 (dragGesture implementation) at REVIEWED_COMMIT. git_grep confirmed no baseOffset/accumulatedOffset/savedOffset exists in the file.
Comment on lines
+90
to
+95
| private var magnifyGesture: some Gesture { | ||
| MagnifyGesture() | ||
| .onChanged { value in | ||
| scale = max(0.2, min(3.0, value.magnification)) | ||
| } | ||
| } |
There was a problem hiding this comment.
🟠 High KnowledgeGraph/KGCanvasView.swift:90
The magnifyGesture resets scale to near-1.0 whenever a new pinch begins because value.magnification always starts at 1.0. If the user is zoomed to 2x and starts another pinch, the zoom jumps back toward 1.0 instead of continuing from 2x. Consider storing a baseScale at gesture start and multiplying it by value.magnification in onChanged.
- private var magnifyGesture: some Gesture {
+ @State private var lastScale: CGFloat = 1.0
+
+ private var magnifyGesture: some Gesture {
MagnifyGesture()
+ .onChanged { value in
+ scale = max(0.2, min(3.0, lastScale * value.magnification))
+ }
+ .onEnded { _ in
+ lastScale = scale
+ }
+ }
+}
- .onChanged { value in
- scale = max(0.2, min(3.0, value.magnification))
- }
- }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/KnowledgeGraph/KGCanvasView.swift around lines 90-95:
The `magnifyGesture` resets `scale` to near-1.0 whenever a new pinch begins because `value.magnification` always starts at 1.0. If the user is zoomed to 2x and starts another pinch, the zoom jumps back toward 1.0 instead of continuing from 2x. Consider storing a `baseScale` at gesture start and multiplying it by `value.magnification` in `onChanged`.
Evidence trail:
brain-bar/Sources/BrainBar/KnowledgeGraph/KGCanvasView.swift lines 90-95 (magnifyGesture implementation), line 7 (@State private var scale), git_grep for baseScale/GestureState/onEnded confirms no baseScale mechanism exists.
Comment on lines
+1475
to
+1484
| while sqlite3_step(stmt) == SQLITE_ROW { | ||
| rows.append(KGEntityRow( | ||
| id: columnText(stmt, 0) ?? "", | ||
| name: columnText(stmt, 1) ?? "", | ||
| entityType: columnText(stmt, 2) ?? "", | ||
| description: columnText(stmt, 3), | ||
| importance: sqlite3_column_double(stmt, 4) | ||
| )) | ||
| } | ||
| return rows |
There was a problem hiding this comment.
🟢 Low BrainBar/BrainDatabase.swift:1475
The while sqlite3_step(stmt) == SQLITE_ROW loop exits on any non-SQLITE_ROW result, including error codes like SQLITE_BUSY or SQLITE_CORRUPT. This silently returns partial results when the database encounters an error mid-iteration. Consider checking sqlite3_errcode(db) after the loop and throwing if it indicates an error rather than SQLITE_DONE.
while sqlite3_step(stmt) == SQLITE_ROW {
rows.append(KGEntityRow(
id: columnText(stmt, 0) ?? "",
name: columnText(stmt, 1) ?? "",
entityType: columnText(stmt, 2) ?? "",
description: columnText(stmt, 3),
importance: sqlite3_column_double(stmt, 4)
))
}
+ let rc = sqlite3_errcode(db)
+ guard rc == SQLITE_DONE || rc == SQLITE_OK else {
+ throw DBError.query(rc)
+ }
return rows🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/BrainDatabase.swift around lines 1475-1484:
The `while sqlite3_step(stmt) == SQLITE_ROW` loop exits on any non-`SQLITE_ROW` result, including error codes like `SQLITE_BUSY` or `SQLITE_CORRUPT`. This silently returns partial results when the database encounters an error mid-iteration. Consider checking `sqlite3_errcode(db)` after the loop and throwing if it indicates an error rather than `SQLITE_DONE`.
Evidence trail:
brain-bar/Sources/BrainBar/BrainDatabase.swift lines 1475-1483 at REVIEWED_COMMIT: shows `while sqlite3_step(stmt) == SQLITE_ROW` loop with `return rows` immediately after, no error checking. SQLite documentation (sqlite.org/c3ref/step.html, sqlite.org/rescode.html) confirms sqlite3_step() can return SQLITE_BUSY, SQLITE_CORRUPT, and other error codes in addition to SQLITE_ROW and SQLITE_DONE.
Comment on lines
+159
to
+164
| func testKGNodeDefaultPosition() { | ||
| let node = KGNode(id: "n1", name: "A", entityType: "person", importance: 5.0) | ||
| // Position should be initialized (not zero — randomized) | ||
| // We just verify the struct is constructable with defaults | ||
| XCTAssertNotNil(node.position) | ||
| } |
There was a problem hiding this comment.
🟢 Low BrainBarTests/KnowledgeGraphTests.swift:159
XCTAssertNotNil(node.position) always passes because position is a CGPoint value type, which can never be nil. This provides no verification that the position is actually randomized rather than zero or any other value.
- func testKGNodeDefaultPosition() {
- let node = KGNode(id: "n1", name: "A", entityType: "person", importance: 5.0)
- // Position should be initialized (not zero — randomized)
- // We just verify the struct is constructable with defaults
- XCTAssertNotNil(node.position)
- }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Tests/BrainBarTests/KnowledgeGraphTests.swift around lines 159-164:
`XCTAssertNotNil(node.position)` always passes because `position` is a `CGPoint` value type, which can never be nil. This provides no verification that the position is actually randomized rather than zero or any other value.
Evidence trail:
brain-bar/Sources/BrainBar/KnowledgeGraph/KGNode.swift lines 9-10: `var position: CGPoint` (non-optional value type)
brain-bar/Tests/BrainBarTests/KnowledgeGraphTests.swift lines 160-165: `testKGNodeDefaultPosition()` test with `XCTAssertNotNil(node.position)` assertion on non-optional CGPoint
Comment on lines
+22
to
+25
| .onAppear { | ||
| viewModel.loadGraph() | ||
| startSimulation() | ||
| } |
There was a problem hiding this comment.
🟡 Medium KnowledgeGraph/KGCanvasView.swift:22
timerActive is set to false in onDisappear but never reset to true in onAppear. If the view reappears without full recreation (e.g., via navigation), startSimulation() is called but the while timerActive loop exits immediately because timerActive remains false. The simulation will not run on subsequent appearances.
.onAppear {
viewModel.loadGraph()
+ timerActive = true
startSimulation()
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/KnowledgeGraph/KGCanvasView.swift around lines 22-25:
`timerActive` is set to `false` in `onDisappear` but never reset to `true` in `onAppear`. If the view reappears without full recreation (e.g., via navigation), `startSimulation()` is called but the `while timerActive` loop exits immediately because `timerActive` remains `false`. The simulation will not run on subsequent appearances.
Evidence trail:
brain-bar/Sources/BrainBar/KnowledgeGraph/KGCanvasView.swift at REVIEWED_COMMIT:
- Line 10: `@State private var timerActive = true` (initialization)
- Lines 20-23: `onAppear` block calls `loadGraph()` and `startSimulation()` but does NOT reset `timerActive`
- Line 24: `.onDisappear { timerActive = false }`
- Lines 96-101: `startSimulation()` contains `while timerActive` loop that will exit immediately if `timerActive` is false
EtanHey
added a commit
that referenced
this pull request
Mar 31, 2026
…over tabs Adds a 3-tab segmented control (Dashboard / Injections / Graph) to StatusPopoverView so users can actually access the InjectionFeedView and KGCanvasView that were merged in PRs #158 and #159 but had no navigation entry point. - PopoverTab enum drives labels, sizes, and tab state - Tabs without dependencies (injectionStore/database) are auto-disabled - Popover resizes per tab via onPreferredSizeChange callback - 13 new tests in PopoverTabTests covering enum, segmented control, tab switching, and backward compatibility Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
6 tasks
EtanHey
added a commit
that referenced
this pull request
Mar 31, 2026
* feat: refresh landing page — new tools, BrainBar section, ecosystem links - Updated tool count from 7 to 11 (added brain_ack, brain_expand, brain_tags, brain_update) - Added BrainBar section showcasing native macOS companion app: Cmd+K search, dashboard, knowledge graph viewer, injection viewer - Updated footer with Golems ecosystem links (cmuxLayer, VoiceLayer) - Added responsive breakpoint for BrainBar grid Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: wire P4 Injection Viewer + P5 Knowledge Graph into BrainBar popover tabs Adds a 3-tab segmented control (Dashboard / Injections / Graph) to StatusPopoverView so users can actually access the InjectionFeedView and KGCanvasView that were merged in PRs #158 and #159 but had no navigation entry point. - PopoverTab enum drives labels, sizes, and tab state - Tabs without dependencies (injectionStore/database) are auto-disabled - Popover resizes per tab via onPreferredSizeChange callback - 13 new tests in PopoverTabTests covering enum, segmented control, tab switching, and backward compatibility Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 2, 2026
This was referenced May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
kg_entitiesandkg_relationswith force-directed physics layout (repulsion + spring + centering forces)kg_entity_chunksBrainDatabase.swiftwithkg_entity_chunkstable, bulk fetch queries, and injection event methods (fixes pre-existing compilation gap)BrainBarSupport.swift(types already defined in dedicated files)New Files
KGNode.swiftKGEdge.swiftKGViewModel.swiftKGCanvasView.swiftKGNodeView.swiftKGEdgeView.swiftKGSidebarView.swiftKnowledgeGraphTests.swiftTest plan
swift test --package-path brain-bar— 205 tests, 0 failuresbash brain-bar/build-app.sh— builds, signs, verifies🤖 Generated with Claude Code
@coderabbitai full review
Summary by CodeRabbit
New Features
Tests
Refactor