-
Notifications
You must be signed in to change notification settings - Fork 0
Handle chunk SSE events for token-by-token streaming #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Greptile SummaryAdded support for token-by-token streaming via a new Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant useChat
participant Server
participant UI
User->>useChat: sendMessage(content)
useChat->>UI: Add user message
useChat->>UI: Create placeholder assistant message
useChat->>Server: sendMessage() via API
Server-->>useChat: SSE: session event
useChat->>useChat: updateSessionId()
loop Token-by-token streaming
Server-->>useChat: SSE: chunk event
useChat->>UI: Append content to message
Note over useChat,UI: content += chunk.content<br/>status = 'streaming'
end
Server-->>useChat: SSE: done event
useChat->>UI: Update status to 'sent'
UI->>User: Display complete message
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 file reviewed, 1 comment
| case 'chunk': { | ||
| // Streaming chunk - append to existing message content | ||
| const data = event.data as { content: string }; | ||
| if (data.content) { | ||
| setMessages((prev) => | ||
| prev.map((m) => | ||
| m.id === messageId | ||
| ? { ...m, content: m.content + data.content, status: 'streaming' as const } | ||
| : m | ||
| ) | ||
| ); | ||
| } | ||
| break; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: 'chunk' is not defined in the SSEEventType union in src/types/index.ts. TypeScript will allow this code but it breaks type safety.
| case 'chunk': { | |
| // Streaming chunk - append to existing message content | |
| const data = event.data as { content: string }; | |
| if (data.content) { | |
| setMessages((prev) => | |
| prev.map((m) => | |
| m.id === messageId | |
| ? { ...m, content: m.content + data.content, status: 'streaming' as const } | |
| : m | |
| ) | |
| ); | |
| } | |
| break; | |
| } | |
| case 'chunk': { | |
| // Streaming chunk - append to existing message content | |
| const data = event.data as { content: string }; | |
| if (data.content) { | |
| setMessages((prev) => | |
| prev.map((m) => | |
| m.id === messageId | |
| ? { ...m, content: m.content + data.content, status: 'streaming' as const } | |
| : m | |
| ) | |
| ); | |
| } | |
| break; | |
| } |
Add 'chunk' to the type definition:
export type SSEEventType =
| 'session'
| 'message'
| 'chunk' // Token-by-token streaming
| 'tool_call'
| 'waiting'
| 'done'
| 'completed'
| 'error';Prompt To Fix With AI
This is a comment left during a code review.
Path: src/hooks/useChat.ts
Line: 131:144
Comment:
**logic:** `'chunk'` is not defined in the `SSEEventType` union in `src/types/index.ts`. TypeScript will allow this code but it breaks type safety.
```suggestion
case 'chunk': {
// Streaming chunk - append to existing message content
const data = event.data as { content: string };
if (data.content) {
setMessages((prev) =>
prev.map((m) =>
m.id === messageId
? { ...m, content: m.content + data.content, status: 'streaming' as const }
: m
)
);
}
break;
}
```
Add `'chunk'` to the type definition:
```typescript
export type SSEEventType =
| 'session'
| 'message'
| 'chunk' // Token-by-token streaming
| 'tool_call'
| 'waiting'
| 'done'
| 'completed'
| 'error';
```
How can I resolve this? If you propose a fix, please make it concise.
No description provided.