-
Notifications
You must be signed in to change notification settings - Fork 0
Add real-time session event monitoring to execution screen #12
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
8365e39
3bb7177
366c947
cb93901
c53ee55
d9709b9
a91c422
81acab6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import { describe, it, expect, vi } from 'vitest'; | ||
| import type { SessionEventData } from './copilot.js'; | ||
|
|
||
| describe('SessionEventData', () => { | ||
| it('should have correct type structure', () => { | ||
| const mockEvent: SessionEventData = { | ||
| type: 'tool.execution_start', | ||
| timestamp: new Date().toISOString(), | ||
| data: { toolName: 'test-tool' }, | ||
| }; | ||
|
|
||
| expect(mockEvent.type).toBe('tool.execution_start'); | ||
| expect(mockEvent.timestamp).toBeDefined(); | ||
| expect(mockEvent.data).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should handle tool.execution_complete events', () => { | ||
| const mockEvent: SessionEventData = { | ||
| type: 'tool.execution_complete', | ||
| timestamp: new Date().toISOString(), | ||
| data: { | ||
| toolCallId: 'test-123', | ||
| success: true, | ||
| result: { content: 'Task completed' }, | ||
| }, | ||
| }; | ||
|
|
||
| expect(mockEvent.type).toBe('tool.execution_complete'); | ||
| expect(mockEvent.data).toHaveProperty('success'); | ||
| }); | ||
|
|
||
| it('should handle session.error events', () => { | ||
| const mockEvent: SessionEventData = { | ||
| type: 'session.error', | ||
| timestamp: new Date().toISOString(), | ||
| data: { | ||
| errorType: 'timeout', | ||
| message: 'Request timed out', | ||
| }, | ||
| }; | ||
|
|
||
| expect(mockEvent.type).toBe('session.error'); | ||
| expect((mockEvent.data as { message: string }).message).toBe('Request timed out'); | ||
| }); | ||
|
|
||
| it('should handle assistant.usage events', () => { | ||
| const mockEvent: SessionEventData = { | ||
| type: 'assistant.usage', | ||
| timestamp: new Date().toISOString(), | ||
| data: { | ||
| model: 'claude-sonnet-4', | ||
| inputTokens: 100, | ||
| outputTokens: 50, | ||
| }, | ||
| }; | ||
|
|
||
| expect(mockEvent.type).toBe('assistant.usage'); | ||
| expect((mockEvent.data as { inputTokens: number }).inputTokens).toBe(100); | ||
| }); | ||
| }); | ||
|
Comment on lines
1
to
60
|
||
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.
Using array index as React key can cause rendering issues when the event log is updated. Since events are bounded to the last 100 and displayed items are sliced from the end, the same index could refer to different events after updates. Consider adding a unique identifier to DisplayEvent (like a counter or timestamp+taskId combination) and use that as the key instead of the array index.
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.
@copilot apply changes based on this feedback
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.
Fixed in d9709b9. Added a unique
idfield toDisplayEventusing a counter-based approach (taskId-timestamp-counter), and updated the map to useevt.idas the React key instead of the array index.