fix(brainbar): KG force-sim CPU pegging (task #22)#249
Conversation
… timerActive reset Root cause of 3 user 'BrainBar stuck' complaints — KGCanvasView.startSimulation pegs CPU at 96.8% because tick() runs a 30fps O(n²) loop with no early-exit and .onAppear doesn't reset timerActive after disappear. Fix: - Energy-threshold early-exit in the force sim — stop timer when KE < 0.01. - Reset timerActive semantics through a restart-safe simulation controller. - Double-start guard for repeated appearances. Tests: stable-graph converges <2s; re-appear triggers restart. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 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 |
![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.
|
@codex review |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| defer { self.simulationTask = nil } | ||
|
|
||
| while self.timerActive && !Task.isCancelled { |
There was a problem hiding this comment.
🟡 Medium KnowledgeGraph/KGSimulationController.swift:36
defer { self.simulationTask = nil } at line 36 nils simulationTask unconditionally when the task completes. If stop() is called (cancelling task A and niling the property) followed immediately by start() (creating task B), task A's defer block still executes and sets simulationTask = nil, discarding the reference to task B. Subsequent stop() calls then fail to cancel task B. Consider removing the defer block and letting stop() exclusively manage the property lifecycle, or capture task identity in the defer to avoid overwriting a newer task.
- simulationTask = Task { @MainActor [weak self] in
- guard let self else { return }
- defer { self.simulationTask = nil }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file brain-bar/Sources/BrainBar/KnowledgeGraph/KGSimulationController.swift around lines 36-38:
`defer { self.simulationTask = nil }` at line 36 nils `simulationTask` unconditionally when the task completes. If `stop()` is called (cancelling task A and niling the property) followed immediately by `start()` (creating task B), task A's defer block still executes and sets `simulationTask = nil`, discarding the reference to task B. Subsequent `stop()` calls then fail to cancel task B. Consider removing the `defer` block and letting `stop()` exclusively manage the property lifecycle, or capture task identity in the defer to avoid overwriting a newer task.
Evidence trail:
brain-bar/Sources/BrainBar/KnowledgeGraph/KGSimulationController.swift lines 31-47 (start method with defer block at line 36), lines 49-53 (stop method). The defer block `defer { self.simulationTask = nil }` unconditionally nils the property without checking task identity, creating the described race condition.
![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: c0f5a61713
ℹ️ 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".
| if tick() < self.energyThreshold { | ||
| self.timerActive = false |
There was a problem hiding this comment.
This branch stops the force loop permanently as soon as one frame reports energy below threshold, but there is no corresponding restart trigger while the canvas stays mounted. KGCanvasView only starts the loop in .onAppear, so after auto-stop any later in-place layout change (notably window resize updating canvasCenter) will not run tick() again and the graph can remain visibly off-center. Please restart the controller when layout-driving state changes, or avoid hard-idling without a resume path.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c0f5a61. Configure here.
| timerActive = true | ||
| simulationTask = Task { @MainActor [weak self] in | ||
| guard let self else { return } | ||
| defer { self.simulationTask = nil } |
There was a problem hiding this comment.
Defer in old task clobbers new simulationTask reference
Medium Severity
When stop() is called followed quickly by start(), the defer { self.simulationTask = nil } in the old task's closure runs after start() has already assigned the new task to simulationTask, clobbering that reference. After this, stop() can no longer cancel the new task via simulationTask?.cancel() because the reference is nil. The simulation still eventually halts via the timerActive flag, but with up to a full 33ms frame of uncancellable lag — undermining the explicit lifecycle control this PR aims to provide.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c0f5a61. Configure here.
1 participant
Summary
Test plan
swift test --filter KGCanvasswift test --filter KGpytestrun observed; failures are pre-existing and unrelated to this Swift-only changeNotes
Pre-existing pytest failures on master unrelated to this Swift-only change: 4 failed + 21 errors in test_engine/test_think_recall (apsw.BusyError) + test_eval_baselines + test_vector_store. Will be tracked separately.
Note
Medium Risk
Touches UI-driven concurrency and the force-simulation loop; bugs could lead to stalled layouts or simulations that don’t stop/restart correctly, but the change is localized and has new tests.
Overview
Reduces Knowledge Graph canvas CPU usage by making the force-directed simulation self-terminating once the layout settles (based on a kinetic-energy threshold) instead of running a perpetual ~30fps loop.
Introduces a
KGSimulationControllerto own the simulation task lifecycle (double-start guard, explicitstop()on view disappear), updatesKGViewModel.tick()to return energy, and adds focused tests covering convergence-to-idle and restart-after-idle behavior.Reviewed by Cursor Bugbot for commit c0f5a61. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix knowledge graph force simulation CPU pegging by auto-stopping on energy convergence
KGSimulationController, a@MainActorcontroller that drives a frame-based loop (33ms per frame), automatically stopping when reported kinetic energy drops below a configurable threshold.KGViewModel.tick()now returns total kinetic energy (CGFloat) so the controller can detect convergence.KGCanvasViewreplaces its manualTask/boolean timer with aKGSimulationControllerinstance;onDisappearcallssimulation.stop()to clean up.📊 Macroscope summarized c0f5a61. 4 files reviewed, 1 issue evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues