fix(ui): restore status indicator during tool calls after text has streamed#89
Merged
ScottKirvan merged 1 commit intoScottKirvan:mainfrom Mar 31, 2026
Conversation
…reamed When an assistant response begins with text before calling tools (e.g. "I'm going to read these files."), assistantEl.setText() was replacing all child nodes including statusEl, leaving it detached. Subsequent onToolCall handlers updated the detached element's text with no visible effect. Fix: introduce a dedicated streamingTextEl span for in-progress text so statusEl remains a sibling. Re-append statusEl in onToolCall if it has been removed. Apply to both handleSend and handleVaultInject response groups. Adds a parseStreamOutput test covering the text→tool→text callback sequence that the DOM fix depends on. Closes ScottKirvan#67 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Summary
The "Thinking…" / tool-status indicator was silently dropped during multi-step responses where Claude emitted text before calling tools.
assistantEl.setText(accumulated)in theonTextcallback replaces the element's entire child list, detachingstatusEl. SubsequentonToolCallhandlers calledstatusEl.setText()on the detached element, so the update was invisible. The indicator only worked reliably when tool calls preceded all text.Issue
Closes #67
Root Cause
statusElis a child<span>ofassistantEl.assistantEl.setText(accumulated)setstextContenton the parent, which is equivalent to removing all child nodes first. After the first text token,statusElis gone from the DOM. The owner's comment ("seems to work sometimes") is explained by the sequence: tool-calls-first responses show the status correctly; text-first responses lose it.Fix
streamingTextElspan insideassistantElfor streaming text, sostatusElremains a sibling rather than being clobbered bytextContentreplacement.onTextnow writes tostreamingTextEl.textContentinstead ofassistantEl.setText().onToolCallre-appendsstatusElviaassistantEl.appendChild(statusEl)if!statusEl.isConnected— handles the case where prior text already removed it.handleSendandhandleVaultInjectresponse groups.assistantEl.empty()+MarkdownRenderer.render()path inonDoneis unchanged; it clears everything and renders final markdown as before.Testing
Regression Risk
parseStreamOutputis unchanged.onDonecallsassistantEl.empty()thenMarkdownRenderer.render()as before; the newstreamingTextElis cleared along with everything else.assistantEl.dataset.markdownis still set fromaccumulatedinonDone— no change.accumulatedinonText; the change is only to which element receives the display update.Notes
isConnectedis a standardNodeproperty (MDN); it is available on all Obsidian-extendedHTMLElementinstances. No new dependencies introduced.