fix(chat): stop the context row re-laying out on every frame - #59
Merged
Conversation
ContextRowOverflowController observes class changes on the context row and ends every layout pass with rowEl.toggleClass(expanded). Obsidian's toggleClass calls classList.add or remove unconditionally, which rewrites the class attribute even when nothing changes, so each pass queued a mutation record that scheduled the next pass. Every chat tab ran this loop once per display frame while Obsidian was visible. The renderer stayed busy, and the cost grew with the number of tabs and with attached note chips. Write the expanded class only when it changes, drop the records a layout pass produced itself, and cancel the pending frame on destroy. The test double for toggleClass now writes unconditionally like Obsidian, and new tests check that a blank or collapsed row stops scheduling frames once it settles and that destroy cancels the pending frame. Add AGENTS.md with this class-helper sharp edge, plus the CLAUDE.md pointer.
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
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.
Intent
With a Qoderian chat tab open, the Obsidian renderer process (Obsidian Helper (Renderer) in macOS Activity Monitor) stays at roughly 24-30% CPU while Obsidian is visible, with elevated GPU helper activity and idle wake-ups. Find the cause and fix it so an open Qoderian tab no longer causes that CPU use, keeping this change limited to the fix and its tests.
What Changed
ContextRowOverflowControllernow callstoggleClassfor the row'sqoderian-context-row--expandedclass only when the value actually changes. Obsidian'stoggleClassrewrites the class attribute even when nothing changes. That write fired the controller's own MutationObserver, which requested another animation frame, so every open Qoderian tab re-ran its context-row layout on every frame, even with no chips attached.mutationObserver.takeRecords()to drop records for DOM it has already read, so the row settles in one frame instead of measuring again a frame later. It also keeps the pendingrequestAnimationFramehandle and cancels it indestroy().toggleClassnow always callsclassList.add/remove, the same way Obsidian's does. New unit tests check that blank and collapsed rows stop requesting animation frames once they settle, and thatdestroy()cancels the pending frame. The Unreleased section ofCHANGELOG.mdgets a Fixed entry.Risk Assessment
✅ Low: The fix is small and contained. It guards the one class write that ran every time and that the MutationObserver reported, which fed the requestAnimationFrame loop running every frame (about 60 times a second). The regression tests check behavior, and the other paths that write into the context row still settle after the fix: the selection pollers, the has-content updater, the file and image chips, and the vault events.
Testing
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
src/features/chat/controllers/context-row-overflow.ts:56- Simplification: the change adds a cancel-on-destroy path. It replaces thelayoutScheduledboolean with alayoutFramehandle (:14, :67-69), adds acancelAnimationFramebranch todestroy()(:56-59), and adds a new test, 'cancels its pending layout frame on destroy' (test :413-426). The CPU goal in the intent doesn't need any of it. Before this change, the existingif (!this.destroyed)check at :70 already turned a frame left pending after destroy into one no-op callback, not a repeating cost. With the cancel added, nothing can run that callback after destroy: both observers are disconnected, the pill is removed, andtoggleExpandedcallslayout()directly. So the change now has two guards for the same condition, and thedestroyedcheck at :70 can never be reached. Remedy: remove the cancel branch, thelayoutFramehandle, and that test, and go back to the boolean flag, unless the author wants frame cancellation on purpose. Runtime impact is negligible either way.⏭️ **Test** - skipped
Step was skipped.
✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.