refactor: streaming architecture with useSignal pattern and mermaid ESM fix#47
Open
JoziGila wants to merge 6 commits intoHaleclipse:masterfrom
Open
refactor: streaming architecture with useSignal pattern and mermaid ESM fix#47JoziGila wants to merge 6 commits intoHaleclipse:masterfrom
JoziGila wants to merge 6 commits intoHaleclipse:masterfrom
Conversation
added 5 commits
January 6, 2026 01:03
Reviewer's GuideRefactors the webview streaming pipeline to use alien-signals-based message/block signals and a centralized StreamingController, while swapping the markdown/mermaid stack to markstream-vue with a custom Mermaid wrapper and relaxed CSP/ESM configuration so diagrams render correctly in VS Code. Sequence diagram for StreamingController-driven message streamingsequenceDiagram
actor User
participant VSCodeExtension as VSCodeExtension
participant WebView as WebviewSession
participant StreamingController as StreamingController
participant Message as Message
participant BlockWrapper as ContentBlockWrapper
participant Vue as VueComponents
User->>VSCodeExtension: Send prompt
VSCodeExtension->>WebView: Stream events (AsyncIterable)
loop For each incoming event
VSCodeExtension->>WebView: event
WebView->>WebView: processIncomingMessage(event)
alt event.type == stream_event
WebView->>StreamingController: handleStreamEvent(streamEvent, parentToolUseId)
alt streamEvent.type == message_start
StreamingController->>Message: createStreaming(parentToolUseId)
Message-->>StreamingController: streaming Message
StreamingController->>StreamingController: createStreamContext()
StreamingController->>Message: setStreaming(true)
StreamingController-->>WebView: onMessageCreated(message, parentToolUseId)
WebView->>WebView: messages([...messages, message])
else streamEvent.type == content_block_start
StreamingController->>StreamingController: get StreamContext
StreamingController->>BlockWrapper: new ContentBlockWrapper(content_block)
StreamingController->>BlockWrapper: startStreaming()
StreamingController->>Message: addBlockToMessage(message, wrapper)
else streamEvent.type == content_block_delta
StreamingController->>StreamingController: get StreamContext
StreamingController->>BlockWrapper: appendDelta(text)
else streamEvent.type == content_block_stop
StreamingController->>StreamingController: get StreamContext
StreamingController->>BlockWrapper: finalizeStreaming()
else streamEvent.type == message_delta
StreamingController->>StreamingController: update usage
StreamingController-->>WebView: onUsageUpdate(usage)
else streamEvent.type == message_stop
StreamingController->>StreamingController: finalize all wrappers
StreamingController->>Message: setStreaming(false)
StreamingController-->>WebView: onMessageFinalized(message, parentToolUseId)
StreamingController->>StreamingController: remove StreamContext
else streamEvent.type == error
StreamingController->>Message: setError(error)
StreamingController->>Message: markInterrupted()
StreamingController->>StreamingController: finalize wrappers and cleanup
end
else Non-stream events
WebView->>WebView: processMessage(event)
WebView->>WebView: processAndAttachMessage(messages, event)
end
WebView-->>Vue: messages signal updated
Vue->>Vue: Re-render AssistantMessage
Vue->>Vue: TextBlock/ThinkingBlock use useSignal(wrapper.text, wrapper.isStreaming)
end
Vue-->>User: Character-by-character streaming text and thinking blocks
Updated class diagram for Message, ContentBlockWrapper, and StreamingControllerclassDiagram
class Message {
+MessageRole type
+MessageData message
+number timestamp
+string subtype
+string session_id
+boolean is_error
+string parentToolUseId
-signal<boolean> streamingSignal
-signal<boolean> interruptedSignal
-string errorMessage
+Message(type: MessageRole, message: MessageData, timestamp: number, extra: any)
+get isStreaming() signal~boolean~
+get isInterrupted() signal~boolean~
+getError(): string
+setStreaming(streaming: boolean): void
+markInterrupted(): void
+setError(error: string): void
+getIsStreaming(): boolean
+getIsInterrupted(): boolean
+disposeStreaming(): void
+get isEmpty(): boolean
+static fromRaw(raw: any): Message
+static createStreaming(parentToolUseId: string): Message
}
class ContentBlockWrapper {
+ContentBlockType content
-signal~ToolResultBlock~ toolResultSignal
-signal~string~ textSignal
-signal~boolean~ streamingSignal
+any toolUseResult
+ContentBlockWrapper(content: ContentBlockType)
+get text() signal~string~
+get isStreaming() signal~boolean~
+appendDelta(delta: string): void
+startStreaming(): void
+finalizeStreaming(finalContent: string): void
+getIsStreaming(): boolean
+getTextValue(): string
+get toolResult() signal~ToolResultBlock~
+setToolResult(result: ToolResultBlock): void
+hasToolResult(): boolean
+getToolResultValue(): ToolResultBlock
-getInitialText(): string
}
class StreamingController {
-Map~string, StreamContext~ activeStreams
-boolean disposed
-Function onMessageCreated
-Function onMessageFinalized
-Function onUsageUpdate
+setOnMessageCreated(callback: Function): void
+setOnMessageFinalized(callback: Function): void
+setOnUsageUpdate(callback: Function): void
+hasActiveStreams(): boolean
+handleStreamEvent(event: any, parentToolUseId: string): void
+cancel(parentToolUseId: string): void
+dispose(): void
-handleMessageStart(event: any, parentToolUseId: string): void
-handleContentBlockStart(event: any, parentToolUseId: string): void
-handleContentBlockDelta(event: any, parentToolUseId: string): void
-handleContentBlockStop(event: any, parentToolUseId: string): void
-handleMessageDelta(event: any, parentToolUseId: string): void
-handleMessageStop(event: any, parentToolUseId: string): void
-handlePing(parentToolUseId: string): void
-handleError(event: any, parentToolUseId: string): void
-createStreamContext(parentToolUseId: string): StreamContext
-disposeStreamContext(context: StreamContext): void
-setTimeoutForContext(context: StreamContext): void
-clearTimeout(context: StreamContext): void
-handleTimeout(context: StreamContext): void
-getCurrentTimeout(context: StreamContext): number
-addBlockToMessage(message: Message, wrapper: ContentBlockWrapper): void
-getBlockType(contentBlock: any): ContentBlockType
}
class StreamContext {
+string parentToolUseId
+Message message
+Map~number, ContentBlockWrapper~ wrappers
+number currentBlockIndex
+ContentBlockType currentBlockType
+Timeout timeoutId
}
Message "1" o-- "*" ContentBlockWrapper : contains
StreamingController "1" o-- "*" StreamContext : manages
StreamContext "1" o-- "1" Message : owns
StreamContext "1" o-- "*" ContentBlockWrapper : tracks
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In
StreamingController.handleContentBlockDelta,delta.partial_jsonis passed directly intoappendDeltawhich expects a string; ifpartial_jsonis an object forinput_json_deltaevents, consider explicitly serializing (e.g.JSON.stringify) or handling that case to avoid[object Object]output or runtime issues. - The
cancelmethod onStreamingControllercurrently disposes the stream context without updating the associatedMessage(e.g.markInterrupted/setStreaming(false)), which may leave the UI in an inconsistent streaming state compared to timeout/error paths; consider aligning cancellation behavior withhandleTimeout/handleError.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `StreamingController.handleContentBlockDelta`, `delta.partial_json` is passed directly into `appendDelta` which expects a string; if `partial_json` is an object for `input_json_delta` events, consider explicitly serializing (e.g. `JSON.stringify`) or handling that case to avoid `[object Object]` output or runtime issues.
- The `cancel` method on `StreamingController` currently disposes the stream context without updating the associated `Message` (e.g. `markInterrupted`/`setStreaming(false)`), which may leave the UI in an inconsistent streaming state compared to timeout/error paths; consider aligning cancellation behavior with `handleTimeout`/`handleError`.
## Individual Comments
### Comment 1
<location> `src/webview/src/core/StreamingController.ts:190-197` </location>
<code_context>
+ /**
+ * Cancel an active stream
+ */
+ cancel(parentToolUseId: string | null = null): void {
+ const context = this.activeStreams.get(parentToolUseId);
+ if (!context) {
+ return;
+ }
+
+ this.disposeStreamContext(context);
+ this.activeStreams.delete(parentToolUseId);
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Canceling a stream does not update the Message streaming/interrupted state, which can leave the UI in an inconsistent state.
Right now `cancel` only disposes the stream context and never updates the `Message` state, so an in-progress message can remain `isStreaming = true` after a user cancel. Please mirror the state updates done in `handleTimeout`/`handleError` (e.g., mark as interrupted or at least not streaming and finalize any active wrappers) before disposing the context.
</issue_to_address>
### Comment 2
<location> `src/webview/src/core/StreamingController.ts:284` </location>
<code_context>
+ }
+
+ // Extract text from delta and append to wrapper
+ const text = delta.text ?? delta.thinking ?? delta.partial_json ?? '';
+ if (text) {
+ wrapper.appendDelta(text);
</code_context>
<issue_to_address>
**issue (bug_risk):** Delta handling assumes a `partial_json` field which may not match the SDK’s actual delta schema.
Given the type definition (`DeltaType = 'text_delta' | 'input_json_delta' | ...`), JSON deltas may come through as `input_json_delta`, but this code only reads `delta.partial_json`. If the SDK’s actual shape uses a different field (e.g. `input_json`, nested `delta`, etc.), those updates will be lost. Consider aligning this with the real SDK schema (e.g. branching on `delta.type`) or handling all known JSON delta shapes explicitly.
</issue_to_address>
### Comment 3
<location> `src/services/webViewService.ts:267-269` </location>
<code_context>
`style-src ${webview.cspSource} 'unsafe-inline' https://*.vscode-cdn.net;`,
`font-src ${webview.cspSource} data:;`,
- `script-src ${webview.cspSource} 'nonce-${nonce}';`,
+ `script-src ${webview.cspSource} 'nonce-${nonce}' 'unsafe-eval' blob:;`,
`connect-src ${webview.cspSource} https:;`,
`worker-src ${webview.cspSource} blob:;`,
].join(' ');
</code_context>
<issue_to_address>
**🚨 issue (security):** Relaxing CSP with `unsafe-eval` and `blob:` for scripts has security implications and might be broader than necessary.
Allowing both `'unsafe-eval'` and `blob:` in the main webview greatly expands what JS can do. If this is only for Mermaid/markstream, can you confirm which directives are strictly required (e.g., do you actually need both, and do they need to apply to the main webview rather than just workers)? Also consider whether this can be limited to dev builds or a narrower origin to keep the CSP as tight as possible in the VS Code webview context.
</issue_to_address>
### Comment 4
<location> `src/webview/src/models/Message.ts:222` </location>
<code_context>
const content = this.message.content;
- // 字符串内容不会是 empty
+ // String content is not empty if has length
if (typeof content === 'string') {
return content.length === 0;
</code_context>
<issue_to_address>
**nitpick:** The comment for the string content branch contradicts the actual emptiness check.
The logic here is correct, but the comment states the opposite of what the code does: it suggests non-empty content, while the branch returns `true` only when `content.length === 0`. Please update or remove the comment so it accurately reflects that this branch returns `true` for empty strings.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
useSignal()pattern (like ToolBlock)alienEffect()+ Vue ref bridge with direct signal consumptionparent_tool_use_idChanges
Streaming Architecture
textSignal,streamingSignalfollowingtoolResultSignalpatternuseSignal(props.wrapper.text)instead of manual effect bridgecreateStreaming()factory, lifecycle methodsMermaid Fix
'unsafe-eval' blob:for mermaid dynamic importsOther
Supersedes
Closes the issues raised in #45 review feedback:
Test plan
🤖 Generated with Claude Code
Summary by Sourcery
Refactor the webview’s assistant streaming architecture to use reactive signals and a centralized streaming controller while integrating a new markdown/mermaid rendering pipeline that works reliably in the VS Code webview.
New Features:
Bug Fixes:
Enhancements:
Build: